Skip to content

bug fix: push :edge when VERSION is updated as well#3662

Merged
georglauterbach merged 1 commit intomasterfrom
release/bugfix-edge
Nov 26, 2023
Merged

bug fix: push :edge when VERSION is updated as well#3662
georglauterbach merged 1 commit intomasterfrom
release/bugfix-edge

Conversation

@georglauterbach
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach commented Nov 26, 2023

Description

Previously, we did not run the workflow on push on master when a release happened because the push on master is guarded by a check on which files were changed.

With this change, I added VERSION to the list of files to consider when updating :edge.

Superseeds #3659

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@georglauterbach georglauterbach added area/ci priority/high kind/bug/fix A fix (PR) for a confirmed bug labels Nov 26, 2023
@georglauterbach georglauterbach self-assigned this Nov 26, 2023
@georglauterbach
push :edge when VERSION is updated
3bdd682 
Previously, we did not run the workflow on push on `master` when a
release happened because the push on master is guarded by a check on
which files were changed.

With this change, I added `VERSION` to the list of files to consider
when updating `:edge`.
@georglauterbach georglauterbach mentioned this pull request Nov 26, 2023
Closed
12 tasks
@georglauterbach georglauterbach enabled auto-merge (squash) November 26, 2023 20:42
@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Nov 26, 2023 •
edited
Loading

Auto-merge is enabled. I have also manually started a run for the default push action, so an updated :edge image will be available around 10pm (UTC+1) (~ +20min from now on).

@georglauterbach georglauterbach added this to the v13.1.0 milestone Nov 26, 2023
casperklein
casperklein approved these changes Nov 26, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@casperklein casperklein left a comment

Choose a reason for hiding this comment

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

BTW: would it hurt, to just build on all changes regardless of the path?

@georglauterbach georglauterbach merged commit 68a43eb into master Nov 26, 2023
@georglauterbach georglauterbach deleted the release/bugfix-edge branch November 26, 2023 20:44
@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Nov 26, 2023 •
edited
Loading

BTW: would it hurt, to just build on all changes regardless of the path?

Probably not; this is somewhat similar to what I was suggesting in #3659 (comment) too.

@georglauterbach
Copy link
Copy Markdown
Member Author

georglauterbach commented Nov 26, 2023 •
edited
Loading

UPDATE: I guess this PR (unexpectedly 😆) solves the issue quite nicely; we should actually not remove paths: to not unnecessarily build an image (e.g., when the documentation is updated). @casperklein

@georglauterbach georglauterbach removed the request for review from polarathene November 26, 2023 20:50
polarathene
polarathene reviewed Nov 27, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

We have a better fix IMO, :edge shouldn't care about this at all.

Comment thread .github/workflows/default_on_push.yml
- .gitmodules
- Dockerfile
- setup.sh
- VERSION # also update :edge when a release happens
Copy link
Copy Markdown
Member

@polarathene polarathene Nov 27, 2023

Choose a reason for hiding this comment

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

The comment is unnecessary:

Suggested change
- VERSION # also update :edge when a release happens
- VERSION

VERSION contributes no meaning to :edge. In the past we have once missed bumping VERSION before tagging a release, but since we only publish images other than :edge by tagged commits, we just published a new tag semver-patch release to resolve that.

I'd rather see the VERSION file removed since it's unnecessary to support the functionality it's intended for, but since it the script in all prior releases since v10 will rely on checking VERSION we'd need to keep it around for a while 🤷‍♂️

If the better update check approach is taken, then the addition to build an image when VERSION changes would become redundant:

Suggested change
- VERSION # also update :edge when a release happens

This was referenced Nov 27, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@polarathene polarathene polarathene left review comments

@casperklein casperklein casperklein approved these changes

Assignees

@georglauterbach georglauterbach

Labels

area/ci kind/bug/fix A fix (PR) for a confirmed bug priority/high

Projects

None yet

Milestone

v13.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@georglauterbach @casperklein @polarathene