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

Push a git tag after a VERSION change #1209

Merged
merged 2 commits into from
May 18, 2020
Merged

Push a git tag after a VERSION change #1209

merged 2 commits into from
May 18, 2020

Conversation

btm
Copy link
Contributor

@btm btm commented May 17, 2020

Normally expeditor does a 'git tag' for us when we use
'built_in:bump_version' but in this case we are doing the version bump
on our own since sometimes we do a major/minor bump based on the the
date.

Aha! Link: https://chef.aha.io/features/SH-1946

@btm btm requested a review from a team as a code owner May 17, 2020 19:49
@netlify
Copy link

netlify bot commented May 17, 2020

Deploy preview for chef-workstation ready!

Built with commit b575936

https://deploy-preview-1209--chef-workstation.netlify.app

@btm btm force-pushed the btm/expeditor-git-tag branch from c7ee3e4 to f7e420e Compare May 17, 2020 19:51
Normally expeditor does a 'git tag' for us when we use
'built_in:bump_version' but in this case we are doing the version bump
on our own since sometimes we do a major/minor bump based on the the
date.

Signed-off-by: Bryan McLellan <btm@loftninjas.org>
@btm btm force-pushed the btm/expeditor-git-tag branch from f7e420e to f876d0a Compare May 17, 2020 19:52
@@ -44,6 +44,9 @@ merge_actions:
- "Expeditor: Skip Omnibus"
- "Expeditor: Skip All"
only_if: bash:.expeditor/determine_version.sh
- bash:.expeditor/push-git-tag.sh:
Copy link
Contributor

Choose a reason for hiding this comment

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

So the tag is on the remote when the omnibus build occurs, we'll want to run this prior to triggering the omnibus build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm presuming the post_commit: true here is redundant?

@btm btm requested a review from tduffield May 18, 2020 17:05
@@ -39,6 +39,8 @@ merge_actions:
ignore_labels:
- "Expeditor: Skip Changelog"
- "Expeditor: Skip All"
- bash:.expeditor/push-git-tag.sh:
Copy link
Contributor

Choose a reason for hiding this comment

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

The order in the config is the order the commands will be executed in, correct? I tried to confirm that in https://expeditor.chef.io/docs/getting-started/action-sets/ but I didn't see it.

If my understanding is correct, looks like BTM got this updated to run before the release.

However, the bash action is pre-commit by default (https://expeditor.chef.io/docs/reference/actions/#bash) so I think we need post_commit: true added back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I actually had it here originally, but with the post-commit: true and expeditor had errored saying that the actions had to be listed in the order they would be executed, which had caused me to move it to the end.

It seems unlikely that the trigger_pipeline:omnibus/release action would be firing before a commit was finished, but it doesn't have a post-commit property on it.

I'm starting to get confused about what commit we're actually referring to.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be before trigger_pipeline, and specified with a post_commit: true. I'll see if I can find out why it was giving you an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

I put your exact scenario into my spec tests and everything came up golden. Let's try your config with the following merge_actions:

            - bash:.expeditor/determine_version.sh:
                ignore_labels:
                  - "Expeditor: Skip Version Bump"
                  - "Expeditor: Skip All"
            - built_in:update_changelog:
                ignore_labels:
                  - "Expeditor: Skip Changelog"
                  - "Expeditor: Skip All"
            - bash:.expeditor/push-git-tag.sh:
                only_if: bash:.expeditor/determine_version.sh
                post_commit: true
            - trigger_pipeline:omnibus/release:
                ignore_labels:
                  - "Expeditor: Skip Omnibus"
                  - "Expeditor: Skip All"
                only_if: bash:.expeditor/determine_version.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

As for "what commit we're actually referring to", there is an implicit commit between the pre-commit and post-commit phases. That's likely what we're referring to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably had it between bash:.expeditor/determine_version.sh: and built_in:update_changelog: before.

Signed-off-by: Bryan McLellan <btm@loftninjas.org>
@btm btm force-pushed the btm/expeditor-git-tag branch from b363dd0 to b575936 Compare May 18, 2020 18:24
Copy link
Contributor

@tduffield tduffield left a comment

Choose a reason for hiding this comment

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

Looks good!

@btm btm merged commit ae3d5ce into master May 18, 2020
@chef-expeditor chef-expeditor bot deleted the btm/expeditor-git-tag branch May 18, 2020 19:29
@btm
Copy link
Contributor Author

btm commented May 18, 2020

looks correct, thanks folks.

btm@btm-thinkpad:~/src/chef-workstation (master)$ git log -n 1
commit 42f02ad470b7e0b37728846fd8a9526c7a6e0bd7 (HEAD -> master, tag: 20.5.42, origin/master, origin/HEAD)
Author: Chef Expeditor <expeditor@chef.io>
Date:   Mon May 18 19:29:50 2020 +0000

    Executed '.expeditor/determine_version.sh'
    
    Obvious fix; these changes are the result of automation not creative thinking.
btm@btm-thinkpad:~/src/chef-workstation (master)$ cat VERSION 
20.5.42

@tyler-ball
Copy link
Contributor

Thanks @btm!

@jonsmorrow jonsmorrow added the Epic label Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants