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

Revert log assignment #209

Closed
wants to merge 1 commit into from

Conversation

kevingentile
Copy link

@kevingentile kevingentile commented Oct 26, 2022

Summary of changes

Revert log assignment to examine a range of commits. The existing command (git show -s --format=%s) only shows a single entry:

$ git show -s --format=%s       
fix log list

with fix (using 1.51.0 as a reference tag):

$ git show -s 1.51.0..HEAD --format=%s
fix log list
Merge pull request #199 from anothrNick/hotfix/sorttags
we introduced a breaking change in outputs rolling back with hotfix
sort tags skip draf releases

Do any of the followings changes break current behaviour or configuration?

  • NO

How changes have been tested

Execution in local bash shell

List any unknowns

n/a

@kevingentile
Copy link
Author

@sbe-arg Does this change look valid? From my end this action at 1.52.0 is improperly tagging pre-release versions:

Setting development pre-tag 2.1.0-development.0 - With pre-tag 2.0.5-development.0
::set-output name=new_tag::2.0.5-development.0
::set-output name=part::pre-patch
::set-output name=tag::2.0.5-development.0
::set-output name=old_tag::2.0.4

@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 27, 2022

The log is not related that output mismatch the log is limited to the last log in order to allow bumps only based on the merge-commit and not in the history was explained in here #199 and here #196 and we had the worst problem with tags is the tagging strangely overwrote 2 tags and deleted the related release with the assoc tag during the CI runs. So the tags had to be manually recreated.

For this reason I strongly advice everyone to use https://github.com/sethvargo/ratchet

Regarding the mismatch Ill have a look shortly

@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 27, 2022

Can you share your GA step config of this action.
And the list of latest tags of your project.

I don't really understand without context what happening but how did you endup in 2.1.0-development.0 from 2.0.4 by #minor comment in pr merge or by automatic bump in action settings or custom_tag?

@sbe-arg sbe-arg added wontfix This will not be worked on invalid This doesn't seem right and removed wontfix This will not be worked on labels Oct 27, 2022
@kevingentile
Copy link
Author

kevingentile commented Oct 27, 2022

Can you share your GA step config of this action. And the list of latest tags of your project.

I don't really understand without context what happening but how did you endup in 2.1.0-development.0 from 2.0.4 by #minor comment in pr merge or by automatic bump in action settings or custom_tag?

Thank you for your time @sbe-arg and apologies for the lack of info here initially.

The bump was triggered from the #minor keyword in a commit message (a commit associated with the merge commit):

...
Merge pull request #999 from my-org/build/bump-minor-version
Bumping #minor version
...

Here are the latest tags :

2.1.0-development.0
2.0.4
2.0.3
2.0.2
2.0.1
2.0.0-development.4
2.0.0-development.3
2.0.0-development.2
2.0.0-development.1
2.0.0-development.0
2.0.0

Action config:

      - name: Determine next tag (develop/other)
        uses: anothrNick/github-tag-action@1.52.0
        if: github.ref != 'refs/heads/develop'
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          DEFAULT_BUMP: patch
          RELEASE_BRANCHES: master,release,develop
          PRERELEASE_SUFFIX: development
          PRERELEASE: true
          DRY_RUN: true

When running the action with v1.52.0, this is the output generated:

*** CONFIGURATION ***
        DEFAULT_BUMP: patch
        WITH_V: false
        RELEASE_BRANCHES: development
        CUSTOM_TAG:
        SOURCE: .
        DRY_RUN: true
        INITIAL_VERSION: 0.0.0
        TAG_CONTEXT: repo
        PRERELEASE: true
        PRERELEASE_SUFFIX: development
        VERBOSE: true
        MAJOR_STRING_TOKEN: #major
        MINOR_STRING_TOKEN: #minor
        PATCH_STRING_TOKEN: #patch
        NONE_STRING_TOKEN: #none
Setting development pre-tag 2.1.0-development.0 - With pre-tag 2.0.5-development.0
::set-output name=new_tag::2.0.5-development.0
::set-output name=part::pre-patch
::set-output name=tag::2.0.5-development.0
::set-output name=old_tag::2.0.4

You mention in #196 and here that this action now looks only for keywords in the merge commit. Additionally, the Pre-release setting now defaults to false where in the past the default was true. Both of these appear to be breaking changes.

@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 27, 2022

Ohhh I see, based on the merge strategy if using squash or rebase it will have 1 or 2 logs during merge but we only read the last one.

The entire thing goes out of wak because we are skipping the commit message bump and is trying to apply the default
here https://github.com/anothrNick/github-tag-action/blob/master/entrypoint.sh#L128 we miss this due the single log
then here https://github.com/anothrNick/github-tag-action/blob/master/entrypoint.sh#L151 [[ "$pre_tag" =~ $new ]] is false hence you get https://github.com/anothrNick/github-tag-action/blob/master/entrypoint.sh#L160-L167 based in https://github.com/anothrNick/github-tag-action/blob/master/entrypoint.sh#L142 wich is minor as default.

Hopefully makes sense.

But preferably we don't want to have the full log of the pr commits only the last few relevant ones. -2 -3 is acceptable.

What if we change it to git show -s --format=%s -2 so it can pickup the # in the second commit.

Sorry I missed this in my testing.

@kevingentile
Copy link
Author

kevingentile commented Oct 27, 2022

@sbe-arg what about a scenario where I begin my branch with a minor version bump:

git commit --allow-empty -m '#minor'
git commit -m 'feature change1'
git commit -m 'feature change2'
git commit -m 'feature change{N}'

The log might look like:

Merge pull request #1234 from my-org/feature-branch
feature change{N}
feature change2
feature change1
#minor

Looking back only 1-2 commits is somewhat arbitrary and feels prone to inconsistency. Any new commits would be relevant.

I have no preference in relying on the keyword in the merge commit only, but it feels like a breaking change that might qualify living in a V2.

@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 27, 2022

The idea is to bump only based on a decision made close to merge not at the beginning.
Have seen scenarios where a PR start as minor, then changes as major but later is reverted to become minor. Because case does not always respect order you might end up with a major bump when you intend a minor along your history.

If we want to maintain order and take only the last one we need a different approach.

We can alternatively make it a new flag in the settings to fit more use cases.
Use full pr history or latest X commits.

@kevingentile
Copy link
Author

The idea is to bump only based on a decision made close to merge not at the beginning. Have seen scenarios where a PR start as minor, then changes as major but later is reverted to become minor. Because case does not always respect order you might end up with a major bump when you intend a minor along your history.

If we want to maintain order and take only the last one we need a different approach.

We can alternatively make it a new flag in the settings to fit more use cases. Use full pr history or latest X commits.

@sbe-arg Makes sense. I can relate to this scenario as we are currently trying to skip a #major keyword that was mistakenly committed to the history.

@sbe-arg sbe-arg closed this Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants