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

Minimal changes for changing global to local #516

Closed

Conversation

bharathappali
Copy link
Collaborator

Issue #511 reflects the reason for daily generation of docker images due to tag concatenation issue, Addressing the review comments I have created this PR considering there will be only one tag passed to build_image function.

I would like to get reviews on this PR if its fine to consider that there will be only one tag getting passed to build_image

This part of script passes only one tag

Signed-off-by: bharathappali [email protected]

@karianna karianna added the bug label Feb 24, 2021
@karianna karianna added this to the February 2021 milestone Feb 24, 2021
@karianna karianna requested a review from dinogun February 24, 2021 09:43
build_latest.sh Show resolved Hide resolved
do
tags="${tags} -t ${repo}:${local_tags[$i]}"
done
tags=" -t ${repo}:${tag}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Is the tags var even referenced anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah at the time when we call check_build_needed here I do accept that with current changes we can pass expanded_tags instead of tags but the check_build_needed expects string to have format -t repo:tag but expanded_tags has a format of -t repo:tag if we update the check_build_needed not to consider space when formating adopt_image_tag we can completely remove the tags var here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your call - I think either leave a comment explaining why they're different or make that change as you suggest.

@karianna karianna modified the milestones: February 2021, March 2021 Mar 1, 2021
@bharathappali
Copy link
Collaborator Author

Closing this PR as #535 fixes it.

@karianna karianna modified the milestones: March 2021, April 2021 Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants