-
Notifications
You must be signed in to change notification settings - Fork 968
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
New lint rule that checks if a Tree
is planted for atleast an app variant.
#381
base: trunk
Are you sure you want to change the base?
Conversation
tikurahul
commented
Feb 20, 2020
- Added tests for the lint rule.
Tree
for planted for an app variant.Tree
is planted for atleast an app variant.
|
||
class TimberIssueRegistry : IssueRegistry() { | ||
override val api = CURRENT_API | ||
override val minApi: Int = CURRENT_API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is new. Not sure we want this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with how androidx writes and publishes lint rules.
Here is an example: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-master-dev:work/workmanager-lint/src/main/java/androidx/work/lint/WorkManagerIssueRegistry.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tor said you should make it as low as you test: #329 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
|
||
override fun afterCheckRootProject(context: Context) { | ||
if (checkForPlantedTrees && !hasPlantedTree && context.driver.phase > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's context.driver.phase > 1
doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a 2 phase lint rule. The first phase checks for the use of Timber's logging APIs. Once we find instances of such use, we start phase 2 (line 52) to ensure that at least one Timber.Tree
was planted.
The afterCheckRootProject
callback is called at the end of every phase, and therefore we only want to report an issue, after 2 phases are complete.
Thanks, Rahul. I'll see if we can get this in now that the project is back on its feet. |
* Added tests for the lint rule.
Once you folks are happy with the PR, I can fix conflicts and re-upload. Please let me know. |