Skip to content

[Nullability Annotations to Java Classes] Add wordpress.lint Dependency#196

Merged
wzieba merged 2 commits intotrunkfrom
deps/add-wordpress-lint-dependency
Oct 25, 2023
Merged

[Nullability Annotations to Java Classes] Add wordpress.lint Dependency#196
wzieba merged 2 commits intotrunkfrom
deps/add-wordpress-lint-dependency

Conversation

@ParaskP7
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 commented Oct 24, 2023

This PR adds wordPress-lint checks to the project (2.0.0).


To test:

  1. Verifying that all the CI checks are successful (focus on the Lint related check and its report).
  2. Verify that the new MissingNullAnnotationOn* correctness related rules are reporting as expected. For a reference, see screenshot below:

image

Release Notes: https://github.com/wordpress-mobile/
WordPress-Lint-Android/releases/tag/2.0.0
Comment thread AutomatticTracks/build.gradle Outdated
url "https://a8c-libs.s3.amazonaws.com/android"
content {
includeGroup "org.wordpress"
includeGroup "org.wordpress.lint"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
includeGroup "org.wordpress.lint"

The group for lint library is org.wordpress - there's no need to add the other group (which is not valid AFAIK, lint is the name of a module)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeap, I wasn't 100% sure about adding org.wordpress.lint too, I just followed the same pattern across all of our other repos, see this Login repo as an example... 🤷

Wdyt @wzieba ? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also to make the example module work on the Catalog repo, I added this line there too, following the Adding a new dependency documentation, as I was indeed hitting a problem on it... 🤷

Build the catalog & example projects - if you get Could not find {dependency} error, make sure the repository for the new dependency you've added is included in example/settings.gradle.kts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeap, I wasn't 100% sure about adding org.wordpress.lint too, I just followed the same pattern across all of our other repos, see this Login repo as an example... 🤷

It seems that this part in Login is also incorrect - it adds a valid group org.wordpress but also invalid and unnecessary one: org.wordpress.fluxc.

Also to make the example module work on the Catalog repo, I added this line there too, following the Adding a new dependency documentation, as I was indeed hitting a problem on it... 🤷

Yes, this line is correct - it adds a new module (not group) so it specifies lint module with it's group: org.wordpress.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it, thanks for the explanation @wzieba ! 🥇

Done: 1e055a6

Related PR Comment: https://github.com/Automattic/
Automattic-Tracks-Android/pull/196#discussion_r1371514709
@wzieba wzieba merged commit d5d89a4 into trunk Oct 25, 2023
@wzieba wzieba deleted the deps/add-wordpress-lint-dependency branch October 25, 2023 11:09
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.

2 participants