-
Notifications
You must be signed in to change notification settings - Fork 329
Read AndroidX Snapshot ID from gradle.properties instead of env #787
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @devbridie, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the mechanism for specifying AndroidX snapshot versions within the project. By transitioning from environment variables to Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request proposes a good change to move the AndroidX snapshot ID configuration from an environment variable to gradle.properties. This makes the build configuration more explicit and version-controllable. My review includes a suggestion to make the new property name more specific to avoid potential naming conflicts in the future, which would improve the long-term maintainability of the build scripts.
|
|
||
| # Use an AndroidX snapshot build. | ||
| # https://androidx.dev/snapshots/builds | ||
| # snapshotVersion=14793336 No newline at end of file |
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.
| @@ -1,4 +1,4 @@ | |||
| val snapshotVersion: String? = System.getenv("COMPOSE_SNAPSHOT_ID") | |||
| val snapshotVersion: String? by settings | |||
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.
The property name snapshotVersion is a bit generic. If you need to add other snapshot repositories in the future (e.g., for Kotlin), it could lead to naming conflicts. Consider renaming it to something more specific like androidxSnapshotVersion to clearly indicate its purpose. This will improve long-term maintainability.
Note that you will also need to update its usage on line 16.
val androidxSnapshotVersion: String? by settings
Hey,
I'd like to propose reading the AndroidX snapshot ID from
settings.gradle.ktsinstead of from the system environment variables. If you switch to snapshot libraries, you need to updatelibs.versions.tomlto switch to-SNAPSHOTversion specifications. That means you should have also checked in the fact that you are swapping to a snapshot repository, which is not possible when reading from System environment variables.I had an internal and external check for usages of COMPOSE_SNAPSHOT_ID, but couldn't find any.