Skip to content

Commit

Permalink
Fix oppia#5329: Add color formatting to static check messages (oppia#…
Browse files Browse the repository at this point in the history
…5540)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes oppia#5329  

This PR implements color-coded messages for static checks to improve
experience. Error messages are displayed in red for failing checks,
while warnings are shown in yellow. Successful checks are indicated with
green messages.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
2 people authored and subhajitxyz committed Nov 19, 2024
1 parent 17e6174 commit fe46540
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 15 deletions.
8 changes: 5 additions & 3 deletions scripts/buf_lint_check.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

source scripts/formatting.sh

jar_file_path=$?
config_file_path=$?
os_type=$?
Expand All @@ -15,11 +17,11 @@ lint_protobuf_files() {
status=$?

if [ "$status" = 0 ] ; then
echo "Protobuf lint check completed successfully"
echo_success "Protobuf lint check completed successfully"
exit 0
else
echo "********************************"
echo "Protobuf lint check issues found. Please fix them before pushing your code."
echo_error "Protobuf lint check issues found. Please fix them before pushing your code."
echo "********************************"
exit 1
fi
Expand Down Expand Up @@ -57,7 +59,7 @@ check_os_type() {
elif [[ "$OSTYPE" == "darwin"* ]]; then
os_type="Darwin"
else
echo "Protobuf lint check not available on $OSTYPE"
echo_error "Protobuf lint check not available on $OSTYPE"
exit 0
fi
}
Expand Down
6 changes: 4 additions & 2 deletions scripts/buildifier_lint_check.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

source scripts/formatting.sh

echo "********************************"
echo "Checking Bazel file formatting"
echo "********************************"
Expand All @@ -19,12 +21,12 @@ $buildifier_file_path --lint=warn --mode=check --warnings=-native-android,+out-o
status=$?

if [ "$status" = 0 ] ; then
echo "Buildifier lint check completed successfully"
echo_success "Buildifier lint check completed successfully"
exit 0
else
# Assume any lint output or non-zero exit code is a failure.
echo "********************************"
echo "Buildifier issue found."
echo_error "Buildifier issue found."
echo "Please fix the above issues."
echo "********************************"
exit 1
Expand Down
6 changes: 4 additions & 2 deletions scripts/checkstyle_lint_check.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

source scripts/formatting.sh

echo "********************************"
echo "Checking Java file formatting"
echo "********************************"
Expand All @@ -23,11 +25,11 @@ echo $lint_results
if [ "$lint_command_result" -ne 0 ] || [ -z "$lint_results" ] || [[ ${lint_results} == *"[WARN]"* ]]; then
# Assume any lint output or non-zero exit code is a failure.
echo "********************************"
echo "Checkstyle issue found."
echo_error "Checkstyle issue found."
echo "Please fix the above issues."
echo "********************************"
exit 1
else
echo "Checkstyle lint check completed successfully"
echo_success "Checkstyle lint check completed successfully"
exit 0
fi
10 changes: 6 additions & 4 deletions scripts/feature_flags_check.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

source scripts/formatting.sh

echo "********************************"
echo "Running feature flag checks"
echo "********************************"
Expand Down Expand Up @@ -120,24 +122,24 @@ function perform_checks_on_feature_flags() {
in_array=$(item_in_array "$element" "${imported_classes[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "$element is not imported in the constructor argument in $file_path at line $imports_line_number"
echo_error "$element is not imported in the constructor argument in $file_path at line $imports_line_number"
fi
done

for element in "${feature_flags[@]}"; do
in_array=$(item_in_array "$element" "${flags_added_to_map[@]}")
if [[ $in_array -ne 1 ]]; then
failed_checks=$((failed_checks + 1))
echo "$element is not added to the logging map in $file_path at line $flags_map_line_number"
echo_error "$element is not added to the logging map in $file_path at line $flags_map_line_number"
fi
done

if [[ $failed_checks -eq 0 ]]; then
echo "Feature flag checks completed successfully"
echo_success "Feature flag checks completed successfully"
exit 0
else
echo "********************************"
echo "Feature flag issues found."
echo_error "Feature flag issues found."
echo "Please fix the above issues."
echo "********************************"
exit 1
Expand Down
30 changes: 30 additions & 0 deletions scripts/formatting.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/bin/bash

#Defines color codes for output formatting

# Red color for error messages
RED='\033[0;31m'

# Green color for success messages
GREEN='\033[0;32m'

# Yellow color for warnings messages
YELLOW='\033[0;33m'

# No color, used to reset the color after each message
NC='\033[0m'

# Function to print an error message in red
function echo_error() {
echo -e "${RED}$1${NC}"
}

# Function to print a success message in green
function echo_success() {
echo -e "${GREEN}$1${NC}"
}

# Function to print a warning message in yellow
function echo_warning() {
echo -e "${YELLOW}$1${NC}"
}
8 changes: 5 additions & 3 deletions scripts/ktlint_lint_check.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/bin/bash

source scripts/formatting.sh

echo "********************************"
echo "Checking code formatting"
echo "********************************"
Expand All @@ -19,15 +21,15 @@ java -jar $jar_file_path --android app/src/**/*.kt data/src/**/*.kt domain/src/*
status=$?

if [ "$status" = 0 ] ; then
echo "Lint completed successfully."
echo_success "Lint completed successfully."
exit 0
else
echo "********************************"
echo "Ktlint issue found."
echo_error "Ktlint issue found."
echo "Please fix the above issues.
You can also use the java -jar $jar_file_path -F --android domain/src/**/*.kt utility/src/**/*.kt data/src/**/*.kt app/src/**/*.kt testing/src/**/*.kt scripts/src/**/*.kt instrumentation/src/**/*.kt
command to fix the most common issues."
echo "Please note, there might be a possibility where the above command will not fix the issue.
echo_warning "Please note, there might be a possibility where the above command will not fix the issue.
In that case, you will have to fix it yourself."
echo "********************************"
exit 1
Expand Down
4 changes: 3 additions & 1 deletion scripts/pre-push.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#!/bin/bash

source scripts/formatting.sh

# This script will run the pre-push checks in the given order
# - ktlint
# - checkstyle
# - buf
# - (others in the future)

if bash scripts/ktlint_lint_check.sh && bash scripts/checkstyle_lint_check.sh && bash scripts/buf_lint_check.sh ; then
echo "All checks passed successfully"
echo_success "All checks passed successfully"
exit 0
else
exit 1
Expand Down

0 comments on commit fe46540

Please sign in to comment.