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

Commits to master branch cause version releases to be messed up #142

Closed
notmessenger opened this issue Mar 5, 2018 · 10 comments
Closed

Comments

@notmessenger
Copy link
Contributor

Whenever a repository has changes made to the master branch (either via 1) changes made directly via the GitHub.com UI, 2) changes made in the master branch and pushed and a PR merged, or 3) changes made in the master branch of a fork and a PR merged (such as in ciena-frost/ember-frost-info-bar#89)) a new MAJOR version is released regardless of the Semver value provided in the PR description and the previous version's CHANGELOG.md contents are used.

See ciena-frost/ember-frost-info-bar#90 as an example of the cause and the problems created and ciena-frost/ember-frost-info-bar#91 as an example of how have to clean it up.

https://github.com/ciena-blueplanet/ciena-devops-testbed can be used to troubleshoot and address this issue.

@job13er
Copy link
Contributor

job13er commented Mar 5, 2018

I'm a little confused by the wording of this issue:

Whenever a repository has changes made to the master branch ... a new MAJOR version is released regardless of the Semver value provided in the PR description and the previous version's CHANGELOG.md contents are used.

This doesn't make much sense to me, and I'm not sure I believe that statement. I'll explain what I expect is happening in the 3 scenarios below.

  1. changes made directly via the GitHub.com UI,

These were never intended to be supported by pr-bumber, there's no PR, so it treats the build on master that is triggered as a merge of the previous PR and the previous PR's scope and changelog would be used again. If the previously merged PR was a MAJOR change, then yes, I would expect a MAJOR bump again, but not if the previous merged PR were a MINOR.

  1. changes made in the master branch and pushed and a PR merged,

I don't quite understand what this means, if changes are made in the master branch and pushed, where is the PR coming into play? Just like changes directly in the GitHub UI, I'd expect pr-bumper to go looking for the last merged PR and use that and apply its scope and changelog data again.

  1. changes made in the master branch of a fork and a PR merged

Do you mean a PR is made from the master branch of a fork to the master branch of the upstream repository? In this case, I would expect things to work fine, in fact, I'm pretty sure it has worked fine in the past, we used to have developers that never created new branches in their forks and always used master.

The commit for the PR you mentioned did not follow the normal format for a merge commit: ciena-frost/ember-frost-info-bar@f50c790 this leads me to believe that your issue in situation 3) is actually a duplicate of: #138 and I'd respond in the same way. Don't use squashed commits with pr-bumper unless you know of an accurate way to find out what the last PR that was merged was in that situation. I haven't used the feature, so I don't know, for instance, if it's safe to assume there will always be the link to the PR in parentheses at the end.

@notmessenger
Copy link
Contributor Author

notmessenger commented Mar 5, 2018

  1. That makes sense. I had only see this occur so far in situations where the previous scope had been MAJOR

  2. You can ignore this

  3. @job13er Can you tell the type of commit that ciena-frost/ember-frost-info-bar@f50c790 is (whether it was a squashed commits)? I've been looking and haven't found it yet. @helrac do you recall which option you chose?

@notmessenger
Copy link
Contributor Author

notmessenger commented Mar 5, 2018

@job13er 1 and 2 - Is there value in modifying pr-bumper so that it either does nothing in this scenario and/or notifies the user that they should not have done this. I'd prefer to prevent the publishing of a new release with the previous scope and CHANGELOG contents with the new code submitted.

@job13er
Copy link
Contributor

job13er commented Mar 5, 2018

  1. Yes. Look at the list of commits, the one for PR Updating coverage info in package.json overwrites the version bump! #89 has the same commit message as the commit from the PR: https://github.com/ciena-frost/ember-frost-info-bar/pull/89/commits

screen shot 2018-03-05 at 2 51 49 pm

@notmessenger
Copy link
Contributor Author

  1. Right, but that doesn't tell us whether it was normal merge or squash merge does it?

@job13er
Copy link
Contributor

job13er commented Mar 5, 2018

1 and 2 - Is there value in modifying pr-bumper so that it either does nothing in this scenario and/or notifies the user that they should not have done this. I'd prefer to prevent the publishing of a new release with the previous scope and CHANGELOG contents with the new code submitted.

I suppose. I wasn't able to figure out a clean way to detect that situation though, which is why I didn't bother implementing it the first time around.

@job13er
Copy link
Contributor

job13er commented Mar 5, 2018

Right, but that doesn't tell us whether it was normal merge or squash merge does it?

I'm pretty sure it does, if it hadn't been, it would say:

Merge pull request #89 from helrac/master

and pr-bumper would have detected it just fine.

@job13er
Copy link
Contributor

job13er commented Mar 5, 2018

Whether it's a merge commit or squash is actually irrelevant, the point is, if it doesn't say

pull request #89

In the commit message pr-bumper won't identify it as a PR merge commit, so that's why it didn't work.

@notmessenger
Copy link
Contributor Author

Thank you for the information about this topic. I have opened #144 to provide guidance in the documentation to avoid taking the actions that cause this scenario since we do not yet know how to prevent this with the tool itself (#142 (comment))

@helrac
Copy link

helrac commented Mar 6, 2018

@notmessenger It was a squash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants