Enhance Linter Github Actions Reporting #4864

ariskotsomitopoulos · 2022-01-05T14:32:45Z

Improve Github Actions Linter reports:

Add ktlint Artifact report

kotlint reports will be also added as comment in the PR
- It will print exactly the lines that have problem
- It will trim the output so the developer will be able to immediately find the class with the error
- Support for all the different modules along with tests
- The same comment will be reused
It is also able to identify the success and print the result appropriately

github-actions · 2022-01-05T15:14:58Z

Unit Test Results

  66 files ±0   66 suites ±0 1m 22s ⏱️ +31s
135 tests ±0 135 ✔️ ±0 0 💤 ±0 0 ❌ ±0
418 runs ±0 418 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 33a4eac. ± Comparison against base commit 1490854.

♻️ This comment has been updated with latest results.

github-actions · 2022-01-05T16:42:17Z

Ktlint Results

👍 ✅ 👍

bmarty

Nice work, thanks! A few little remarks

bmarty · 2022-01-05T21:14:37Z

.github/workflows/quality.yml

+          else
+            body="\`👎 ❌ 👎 ${results}\`"
+          fi
+          body="${body//'%'/'%25'}"


I would apply all the treatment you are doing on the else block above and on the results var, before affecting the value prefixed with emojis to body.

Yes I agree, it's really sensitive, if it can support multiple commands inside the else block I will change it

I see, thanks

bmarty · 2022-01-05T21:15:40Z

.github/workflows/quality.yml

+          if [ -z  "$results" ]; then
+            body="👍 ✅ 👍"
+          else
+            body="\`👎 ❌ 👎 ${results}\`"


Small remark, the Emojis could be outside of the "`", no?

Sure it would, however if you observe commenting with that lib do not support \n so I had to manually create the new lines by replacing via bash the aggregated results with <br>. But then I realised that code blocks do not support
new lines! 😵‍💫 So to support newlines along with code blocks I added multiple code blocks around <br>. So there should be an init left_code_block in order to work.

It was the tricky to support newlines with codeblocks without the lib supporting it

The best I can do is (that seems pretty nice):
👎 ❌ 👎Failed

Ah, thanks for the explaination.
Maybe do not add Emoji there but use the reactions, different depending on the result?
Anyway this is fine like this.

bmarty · 2022-01-05T21:16:34Z

.github/workflows/quality.yml

+          comment-author: 'github-actions[bot]'
+          body-includes: Ktlint Results
+      - name: Publish ktlint results to PR
+        if: always()


OOI, is there a necessity to add this line?

Yes it is an important line, while when ktlint fails thats when we should publish the results, also that was the fix for the artifacts. The problem was that the jobs was killed so nothing happened after that

Ok, understood, thanks!

bmarty · 2022-01-05T21:17:59Z

.github/workflows/quality.yml


-# Lint for main module and all the other modules
+            ${{ steps.get-comment-body.outputs.body }}
+          reactions: rocket


I do not see the added value to automatically add a reaction to the generated comment (except that it is nice to know that it is possible to do so)

haha yes I just wanted to test it, I will remove it

- Publish results on PR via commenting - Support for all modules along with tests - Code format & emoticons

ouchadam · 2022-01-07T11:43:21Z

awesome change! 💯

would love to see a v2 that has url clickthroughs for the failing lines 🤩

bmarty · 2022-01-07T13:14:10Z

.github/workflows/quality.yml

@@ -74,7 +118,6 @@ jobs:
        run: ./gradlew clean lint${{ matrix.target }}Release --stacktrace
      - name: Upload ${{ matrix.target }} linting report
        uses: actions/upload-artifact@v2
-        if: always()


Just a last remark, will the lint report be uplodoad if the command fails (so if there are lint issue) if you remove this line?

(related to your comment https://github.com/vector-im/element-android/pull/4864/files#r779175980)

Edit:
Here is a different case than before because the way command ./gradlew clean lint run needs to be succeed in order to generate the report! So there is no need to upload an artifact without the report. In comparison to ktlint that the report is generated when the build fails. Is it clear now?

Yes, thanks!

element-hq deleted a comment from github-actions bot Jan 5, 2022

ariskotsomitopoulos force-pushed the feature/aris/linter_github_actions_improvement branch 3 times, most recently from b39e9b0 to 6b85110 Compare January 5, 2022 20:55

bmarty reviewed Jan 5, 2022

View reviewed changes

ariskotsomitopoulos marked this pull request as ready for review January 5, 2022 21:49

- Fix github actions ktlint reports

33a4eac

- Publish results on PR via commenting - Support for all modules along with tests - Code format & emoticons

ariskotsomitopoulos force-pushed the feature/aris/linter_github_actions_improvement branch from aad7dff to 33a4eac Compare January 5, 2022 22:27

ouchadam approved these changes Jan 7, 2022

View reviewed changes

bmarty reviewed Jan 7, 2022

View reviewed changes

bmarty approved these changes Jan 7, 2022

View reviewed changes

bmarty merged commit bcebfff into develop Jan 7, 2022

bmarty deleted the feature/aris/linter_github_actions_improvement branch January 7, 2022 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Linter Github Actions Reporting #4864

Enhance Linter Github Actions Reporting #4864

ariskotsomitopoulos commented Jan 5, 2022 •

edited

Loading

github-actions bot commented Jan 5, 2022 •

edited

Loading

github-actions bot commented Jan 5, 2022 •

edited

Loading

bmarty left a comment

bmarty Jan 5, 2022

ariskotsomitopoulos Jan 5, 2022

bmarty Jan 7, 2022

bmarty Jan 5, 2022

ariskotsomitopoulos Jan 5, 2022

bmarty Jan 5, 2022

bmarty Jan 5, 2022

ariskotsomitopoulos Jan 5, 2022 •

edited

Loading

bmarty Jan 7, 2022

bmarty Jan 5, 2022

ariskotsomitopoulos Jan 5, 2022

ouchadam commented Jan 7, 2022 •

edited

Loading

bmarty Jan 7, 2022

bmarty Jan 7, 2022

ariskotsomitopoulos Jan 7, 2022 •

edited

Loading

bmarty Jan 7, 2022

Enhance Linter Github Actions Reporting #4864

Enhance Linter Github Actions Reporting #4864

Conversation

ariskotsomitopoulos commented Jan 5, 2022 • edited Loading

github-actions bot commented Jan 5, 2022 • edited Loading

Unit Test Results

github-actions bot commented Jan 5, 2022 • edited Loading

Ktlint Results

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ariskotsomitopoulos Jan 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam commented Jan 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ariskotsomitopoulos Jan 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ariskotsomitopoulos commented Jan 5, 2022 •

edited

Loading

github-actions bot commented Jan 5, 2022 •

edited

Loading

github-actions bot commented Jan 5, 2022 •

edited

Loading

ariskotsomitopoulos Jan 5, 2022 •

edited

Loading

ouchadam commented Jan 7, 2022 •

edited

Loading

ariskotsomitopoulos Jan 7, 2022 •

edited

Loading