-
Notifications
You must be signed in to change notification settings - Fork 130
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
Implement Environment Variables for Maven and GraalVM Versions in Dockerfile #402
Implement Environment Variables for Maven and GraalVM Versions in Dockerfile #402
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe Dockerfile has been updated to introduce flexibility in version management for Maven and GraalVM by utilizing environment variables. This change simplifies the update process for these tools and enhances the capability to test the application with different versions, aligning with the goal of maintainable and adaptable development setup. Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
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.
Yeah! You did it 🎉 Now, Relax 😉, Grab a drink ☕, and wait for the maintainers to check your contributions. Meanwhile, you can discuss on other issues and solve them 😀. Thank You 😃!
Meanwhile you can also discuss about the project in our Discord Server 😀
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- docker-compose.yaml
Files selected for processing (1)
- Docker/dev/commons/base/Dockerfile (1 hunks)
Additional comments: 6
Docker/dev/commons/base/Dockerfile (6)
14-14: The use of the
MAVEN_MAJOR
andMAVEN_VERSION
variables in thewget
command is a good practice for maintainability, but ensure that these variables are defined either in the Dockerfile or passed as build arguments.15-15: Similar to the Maven installation, verify that the
GRAAL_VERSION
variable is properly defined or passed to the Docker build context to avoid build errors.18-19: Creating symbolic links for Maven and GraalVM is a good practice for consistency and ease of use. However, ensure that these links do not conflict with any existing directories or links in the
/opt
directory.20-21: Removing the downloaded archive files is a good practice to reduce the Docker image size. This is a clean-up step that should be maintained.
24-24: Updating the
PATH
environment variable to include Maven's bin directory is correct. This ensures that Maven commands can be run from any location within the container.25-26: Setting
GRAALVM_HOME
andJAVA_HOME
to the GraalVM installation directory is appropriate. This ensures that Java-based applications can locate the correct JVM.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- Docker/dev/commons/base/Dockerfile (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- Docker/dev/commons/base/Dockerfile
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.
@Surya-KN The Maven download link does not get resolved. Please fix this issue.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- docker-compose.yaml
Files selected for processing (1)
- Docker/dev/commons/base/Dockerfile (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- Docker/dev/commons/base/Dockerfile
I have made the changes, check it out @SaptarshiSarkar12 |
@Surya-KN Still not working |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- docker-compose.yaml
Files selected for processing (1)
- Docker/dev/commons/base/Dockerfile (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- Docker/dev/commons/base/Dockerfile
|
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.
@Surya-KN Looks good to merge 👍.
Thanks for contributing 🚀 🚀.
You may join our Discord server - https://discord.gg/DeT4jXPfkG to get updates about the project.
Yeah, it's working now. |
Fixes issue
Fixes #401
Changes proposed
Modified the Dockerfile to use environment variables for the Maven and GraalVM versions. This allows for easy version updates by simply changing the environment variables, eliminating the need for Dockerfile modifications.
The changes in this pull request will enhance the maintainability of Docker setup and streamline the process of testing application with different versions of Maven and GraalVM.
Check List (Check all the applicable boxes)
Screenshots
Note to reviewers
Summary by CodeRabbit
Chores
Documentation