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

feat: begin work on adding Ktlint / Android build step to the repo #1152

Closed
wants to merge 4 commits into from

Conversation

TheRogue76
Copy link
Collaborator

@TheRogue76 TheRogue76 commented Jan 5, 2024

This PR aims to add Ktlint to the repo and maybe setup some extra build steps for Android like we have in iOS. This is an effort to make sure stuff like #1150 can never happen again.

@TheRogue76 TheRogue76 changed the title feat: begin work on adding Ktlint to the repo feat: begin work on adding Ktlint / Android build step to the repo Jan 5, 2024
@TheRogue76
Copy link
Collaborator Author

Let's see if https://github.com/lottie-react-native/lottie-react-native/actions/runs/7427881570/job/20214305445 fails. If it does, i'll add it to the base build steps in CI and remove the trigger so it runs on every PR

@TheRogue76
Copy link
Collaborator Author

TheRogue76 commented Jan 6, 2024

It doesn't. gradle assemble straight up ignores it, it's only gradle build that calls lottie-react-native:lint. The linter is ours and already existed as one of our verification gradle tasks, it wasn't a third party linter. @matinzd

I can rename our build:android jobs to assemble:android (because that is what they are actually doing) and add a new job to actually build android, or run ./gradlew lottie-react-native:lint if we want it to be faster and combine the ktlinter job and that job together, but it needs to be done in the apps, because if we run it in core, it is gonna error out due to not seeing dependencies like react native and others

@TheRogue76
Copy link
Collaborator Author

Scrapping the PR, will do another one, with the linter gradle command only

@TheRogue76 TheRogue76 closed this Jan 7, 2024
@TheRogue76 TheRogue76 deleted the feature/add-ktlint branch January 8, 2024 16:45
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.

None yet

1 participant