Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send Android lint reports to GitHub Code Scanning #21207

Merged
merged 10 commits into from
Sep 19, 2024
Merged

Conversation

wzieba
Copy link
Contributor

@wzieba wzieba commented Sep 5, 2024

Description

This PR introduces integration with GitHub Code Scanning feature for Android lint.

Internal project reference: paaHJt-7ay-p2

To Test:

Check: https://github.com/wordpress-mobile/WordPress-Android/security/code-scanning?page=1&query=is%3Aopen+branch%3Atrunk+pr%3A21207 and verify there are issues reported by Android lint.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 5, 2024

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21207-cc99dd3
Commitcc99dd3
Direct Downloadjetpack-prototype-build-pr21207-cc99dd3.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 5, 2024

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21207-cc99dd3
Commitcc99dd3
Direct Downloadwordpress-prototype-build-pr21207-cc99dd3.apk
Note: Google Login is not supported on these builds.

lint_exit_code=$?
set -e

upload_sarif_to_github "WordPress/build/reports/lint-results-wordpressVanillaRelease.sarif"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't send 2 report files, so I decided to stick to WordPress one for no specific reason over Jetpack.

Copy link
Contributor

@ParaskP7 ParaskP7 Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I were to chose between WordPress or Jetpack @wzieba , I would personally chose Jetpack instead. I would do so because the WordPress app is a subset of the Jetpack app and doesn't contain every feature. Having said that, I understand that the codebases is almost fully shared between the two and that those Jetpack features not available to WordPress is behind a feature flag or an if/else statement of some kind, located on the main sourceset, which means, it applies to both apps. Still, I think, even conceptually, if I had the choice, I would chose Jetpack. 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha - thanks! I've updated in cc99dd3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome thanks! 🚀

@wzieba wzieba added this to the 25.7 milestone Sep 18, 2024
@wzieba wzieba changed the title [wip] Send sarif reports to GH registry Send Android lint reports to GitHub Code Scanning Sep 18, 2024
@wzieba wzieba marked this pull request as ready for review September 18, 2024 16:13
@wzieba wzieba requested a review from ParaskP7 September 18, 2024 16:13
As WordPress is a subset of Jetpack
Copy link

Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 @wzieba !

I have reviewed and also tested (#21245) this PR, everything seems to be working as expected, awesome job on introducing this new workflow, in this and every other client project! 🌟


Additionally, I suggested using Jetpack over WordPress, if that's worth it of a quick change, but nothing blocking (context).

@wzieba wzieba merged commit a5f14e7 into trunk Sep 19, 2024
21 checks passed
@wzieba wzieba deleted the send_sarif_reports_to_gh branch September 19, 2024 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants