-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #11397 - Add lint rule for AppCompatResources instead of ContextCompat #16011
Conversation
No Taskcluster jobs started for this pull requestThe `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request. |
mozilla-lint-rules/build.gradle
Outdated
@@ -15,6 +15,7 @@ dependencies { | |||
|
|||
testImplementation "com.android.tools.lint:lint:${Versions.android_lint_api}" | |||
testImplementation "com.android.tools.lint:lint-tests:${Versions.android_lint_api}" | |||
testImplementation 'junit:junit:4.12' |
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.
I think we can just do testImplementation Deps.junitApi
(Just pulled this into #16041 to make sure the other license tests can run)
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.
Great!
I checked out the Versions
object and it had JUnit 5 so I went for the hardcoded version (definitely expecting a review comment), didn't check the Deps
object though
I'll update it to testImplementation Deps.junitApi
then :)
@@ -0,0 +1,159 @@ | |||
package org.mozilla.fenix.lintrules |
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.
nit - license (haha maybe the linter would have caught this :P )
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.
Oh lol, I missed it (I guess I don't see the top of my files often haha)
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.
Hm, I'm not getting the LicenseDetector
warning on test files, maybe there's something I need to configure?
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.
2 nits!
Done! @ekager |
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.
looks good! :)
So looks like there are 6 lint errors where we are using the wrong thing here - would you mind fixing those here too? :) |
Of course, just fixed those 6 calls (reported by the linter) :) |
Codecov Report
@@ Coverage Diff @@
## master #16011 +/- ##
=========================================
Coverage 29.86% 29.86%
+ Complexity 1191 1190 -1
=========================================
Files 453 453
Lines 18535 18535
Branches 2407 2407
=========================================
Hits 5535 5535
Misses 12606 12606
Partials 394 394
Continue to review full report at Codecov.
|
Looks good, let's land it! |
Pull Request checklist
To download an APK when reviewing a PR: