Skip to content
This repository was archived by the owner on Jul 28, 2026. It is now read-only.

Fix issue with v0.0.0 builds. - #832

Merged
mattdurham merged 2 commits into
mainfrom
drone_tagging
Aug 18, 2021
Merged

Fix issue with v0.0.0 builds.#832
mattdurham merged 2 commits into
mainfrom
drone_tagging

Conversation

@mattdurham

Copy link
Copy Markdown
Contributor

PR Description

This will fix the issue with v0.0.0 versions being pushed.

Which issue(s) this PR fixes

Closes #823

Notes to the Reviewer

PR Checklist

  • CHANGELOG updated

@mattdurham
mattdurham marked this pull request as ready for review August 18, 2021 13:46
@mattdurham
mattdurham requested a review from rfratto August 18, 2021 13:46
Comment thread Makefile
ifeq ($(BUILD_IN_CONTAINER),true)
docker build -t windows_installer ./packaging/windows
docker run --rm -t -v "${PWD}:/home" windows_installer
docker run --rm -t -v "${PWD}:/home" -e VERSION=${RELEASE_TAG} windows_installer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, can't we use ${IMAGE_TAG} here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are equivalent here, we could likely clean up to only use one of them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Though there is a situation where you can manually override each one and they would be different, not even sure if that's a valid case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I'm not sure that's something we'd want to explicitly support

@rfratto rfratto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, up to you whether you want to try removing the reference to RELEASE_TAG when building the Windows installer

@mattdurham
mattdurham merged commit c46b550 into main Aug 18, 2021
@mattdurham
mattdurham deleted the drone_tagging branch August 18, 2021 14:01
mattdurham added a commit that referenced this pull request Nov 11, 2021
@github-actions github-actions Bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Apr 11, 2024
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drone: merges to main not tagging images properly

2 participants