-
Notifications
You must be signed in to change notification settings - Fork 0
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
update version #3
Conversation
Dockerfile
Outdated
@@ -1,8 +1,9 @@ | |||
FROM blcdsdockerregistry/bl-base:1.0.0 AS builder | |||
ARG MINIFORGE_VERSION=22.9.0-2 |
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.
Is this the latest version?
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.
Nope. Updated to latest mambaforge: v23.1.0-1.
Dockerfile
Outdated
# Change the default user to bldocker from root | ||
USER bldocker | ||
|
||
LABEL maintainer="Helena Winata <[email protected]>" \ |
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.
Do you want to update the maintainer as well since the yaml was updated?
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.
updated and retested. Ready to merge?
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.
@sorelfitzgibbon It looks like we're missing the auto-build github action? Can you check the template and add any missing files?
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 to me. I'll leave the final approval to @yashpatel6.
metadata.yaml
Outdated
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.
We'll want to add image_name
in metadata.yaml for the auto-building action to name the image
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.
@yashpatel6, done.
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!
@sorelfitzgibbon can you also put the image here
|
@tyamaguchi-ucla, boutrosuser/group doesn't have write permission:
|
I've updated the permissions now. Also, on a side note, since this isn't automated for packages, we'll want to add a function to the BL util similar to |
Checklist
Formatting
I have read the code review guidelines and the code review best practice on GitHub check-list.
The name of the branch is meaningful and well formatted following the standards, using [AD_username (or 5 letters of AD if AD is too long)-[brief_description_of_branch].
I have set up or verified the branch protection rule following the github standards before opening this pull request.
File Updates
I have ensured that the version number update follows the versioning standards.
I have updated the version number/dependencies and added my name to the maintainer list in the
Dockerfile
.I have updated the version number/feature changes in the
README.md
.I have updated the version number and added my name to the contributors list in the
metadata.yaml
.I have added the changes included in this pull request to the
CHANGELOG.md
under the next release version or unreleased, and updated the date.CHANGELOG.md
in the release.Docker Image Testing
docker run
command as described below.My command:
Output: