-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add github action to generate tags and diff #80
Conversation
github-token: ${{ secrets.GITHUB_TOKEN }} | ||
script: | | ||
commentText = 'Diff for ' + context.payload.pull_request.head.sha + ':\n\n' | ||
needNewComment = true; |
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.
Can you add some comments on what this is doing in the different blocks?
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.
Added. Some of the descriptions are available in console.log
's as well.
@@ -0,0 +1,41 @@ | |||
import json | |||
|
|||
DEFAULT_ALPINE_VERSION = '3.12' |
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 should update to a newer base version since 3.12 goes out of support relatively soon.
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.
Yes, I will be creating a PR once this is merged to address this.
bin/tag-generator.py
Outdated
print("Architectures: amd64, arm64v8") | ||
print(f"Directory: {key}/jdk/al2\n") | ||
|
||
for alpine_version in ['3.12', '3.13', '3.14']: |
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.
Can we pull this list up to a top level var?
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.
Done.
@@ -10,16 +12,15 @@ update_musl_linux() { | |||
CORRETTO_VERSION=$1 | |||
MAJOR_RELEASE=$2 | |||
|
|||
sed -i "" "s/ARG version=.*/ARG version=${CORRETTO_VERSION}/g" ./${MAJOR_RELEASE}/jdk/alpine/Dockerfile | |||
${SED} "s/ARG version=.*/ARG version=${CORRETTO_VERSION}/g" ./${MAJOR_RELEASE}/jdk/alpine/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.
I think we should create a backlog item to change this to use templates and not a ton of sed commands.
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.
Created Issue #81 for this.
Description of changes:
Add Github actions to generate tags for docker library and validate that the content of the repository is the expected content.
Sample of the content available at PR : cliveverghese#6