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

build: Add compose lints to componentsv2 #170

Open
wants to merge 1 commit into
base: jonny/DCMAW-8694-idiomatic-compose
Choose a base branch
from

Conversation

jonnyandrew
Copy link
Contributor

@jonnyandrew jonnyandrew commented Jan 16, 2025

Add Slack's compose-lints lint rules to componentsv2.

These checks are to ensure that your composables don’t fall into common pitfalls that may be easy to miss in code reviews.

Evidence of the change

Checklist

Before creating the pull request

  • Commit messages that conform to conventional commit messages.
  • Ran the app locally ensuring it builds.
  • Tests pass locally.
  • Pull request has a clear title with a short description about the feature or update.
  • Created a draft pull request if it's not ready for review.

Before the CODEOWNERS review the pull request

  • Complete all Acceptance Criteria within Jira ticket.
  • Self-review code.
  • Successfully run changes on a testing device.
  • Complete automated Testing:
    • Unit Tests.
    • Integration Tests.
    • Instrumentation / Emulator Tests.
  • Review Accessibility considerations.
  • Handle PR comments.

Before merging the pull request

@jonnyandrew jonnyandrew mentioned this pull request Jan 17, 2025
16 tasks
@jonnyandrew jonnyandrew changed the base branch from jonny/DCMAW-8694-idiomatic-compose to main January 17, 2025 10:00
@jonnyandrew jonnyandrew changed the base branch from main to jonny/DCMAW-8694-idiomatic-compose January 17, 2025 10:00
<issues format="6" by="lint 8.8.0" type="baseline" client="gradle" dependencies="false" name="AGP (8.8.0)" variant="all" version="8.8.0">

<issue
id="ComposeModifierReused"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These particular problems should not be ignored. I added to the baseline so that future work can benefit from the lint check.

Comment on lines +108 to +112
android {
lint {
baseline = file("lint-baseline.xml")
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed due to a bug in mobile-android-pipelines where the lint baseline file is not configured to support a multi-module setup.

@jonnyandrew jonnyandrew marked this pull request as ready for review January 17, 2025 10:15
@jonnyandrew jonnyandrew requested review from a team as code owners January 17, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant