Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make v prefix for tags optional #420

Closed
wants to merge 3 commits into from
Closed

Make v prefix for tags optional #420

wants to merge 3 commits into from

Conversation

abigailwillow
Copy link

@abigailwillow abigailwillow commented Aug 2, 2022

Description

There is no mention of requiring a prefix to the tags in the documentation. This is unexpected behavior, and as a result of this requirement tags such as 0.1.0 do not work with the semantic versioning strategy. Additionally, the semantic versioning documentation referenced has not used the prefix for a couple of years.

Changes

  • Removed the requirement to have a v prefixing tags.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

Cat Gif

@abigailwillow abigailwillow changed the title Remove v prefix from tag checking Remove v prefix from tag checking Aug 2, 2022
@abigailwillow abigailwillow changed the title Remove v prefix from tag checking Remove v prefix from tag checking Aug 2, 2022
@webbertakken
Copy link
Member

Hi @abbydiode, thank you for raising a PR to improve things!

Agreed that this is somewhat confusing.

However we determined some time ago that it is convention for version tags on GitHub start with a v.
Since this has worked well for a few years now and because this would introduce a breaking change I believe it's better to update the documentation instead of the behaviour itself.

@abigailwillow
Copy link
Author

However we determined some time ago that it is convention for version tags on GitHub start with a v. Since this has worked well for a few years now and because this would introduce a breaking change I believe it's better to update the documentation instead of the behaviour itself.

@webbertakken thank you for your feedback. I thought the way the changes were made the v prefix would be optional. However I realized afterwards that I had made a mistake with the matching pattern. This has been resolved in the current state of this PR, meaning the v prefix is now optional.

@webbertakken
Copy link
Member

I really appreciate the effort, but as mentioned it might be better to fix the documentation instead of the behaviour because it worked well (other than the confusion you mentioned) and the change would introduce a breaking change: anyone who upgrades to the new version will now have to remove all non v version tags that start with a number if they weren't intended for versioning.

Closing for that reason.

@abigailwillow abigailwillow changed the title Remove v prefix from tag checking Make v prefix for tags optional Aug 2, 2022
@connieprice
Copy link

I really appreciate the effort, but as mentioned it might be better to fix the documentation instead of the behaviour because it worked well (other than the confusion you mentioned) and the change would introduce a breaking change: anyone who upgrades to the new version will now have to remove all non v version tags that start with a number if they weren't intended for versioning.

Closing for that reason.

I don't see how this is a breaking change, or where including v is a GitHub standard. Making 'v' optional should not break any existing workflows, and would allow for new workflows to follow the semantic versioning standard more closely.

@abigailwillow
Copy link
Author

I believe that if a users assigns for example 1.2.3 as a tag to a commit it would be expected behavior for the builder's semantic versioning to pick up on that. Not only if they assign for example v1.2.3.

Additionally I would like to add that the FAQ for semantic versioning specifically mentions that anything including v is not a semantic version:

Is “v1.2.3” a semantic version?
No, “v1.2.3” is not a semantic version. However, prefixing a semantic version with a “v” is a common way (in English) to indicate it is a version number. Abbreviating “version” as “v” is often seen with version control. Example: git tag v1.2.3 -m "Release version 1.2.3", in which case “v1.2.3” is a tag name and the semantic version is “1.2.3”.

@webbertakken
Copy link
Member

webbertakken commented Aug 2, 2022

I understand where you're coming from. We've had this same discussion a few times before on our Discord.

Semantic version vs version tags
Don't get me wrong, I'm not saying v1.2.3 is a semantic version. I'm saying it's most commonly used here on github to denote the tag holds a (semantic) version number (after the v). I also didn't invent that, but years ago we decided to follow that best practice with unity-builder.

Why is it a breaking change?
Please note that this action is used by many projects. Suddenly counting other commits additional tags to count for versioning may mess with people's existing workflows. Therefore we'd have to mark it as a breaking change and bump major for it. Note that asking people to remove existing tags might also be problematic.

@webbertakken
Copy link
Member

Just looking through history and it looks like we already voted in favour of making the v optional afterall.

#303

What you're looking for already works as expected.

@abigailwillow
Copy link
Author

Please note that this action is used by many projects. Suddenly counting other commits to count for versioning may mess with people's existing workflows. Therefore we'd have to mark it as a breaking change and bump major for it. Note that asking people to remove existing tags might also be problematic.

Sorry, I'm not sure if I understand why users would have to count 'other commits' for versioning and why users would have to remove existing tags. All this change would introduce is the v prefix being optional or not.

To me the fact that only tags with the v prefix are counted to increase the major/minor version with the semantic versioning strategy is problematic for the following reasons:

  1. It is not obvious from the documentation that the v prefix is necessary, therefore it may look to users like this versioning strategy is broken.
  2. When requiring the v prefix your list of tags will get cluttered with manually assigned major/minor increase tags that have the v prefix. Whereas the output from the build step does not have the v prefix. To rectify this in a CI pipeline users will always have to include the v in their actions.

Even if this would be considered a breaking change, wouldn't it be possible to create a 'new' versioning strategy that's essentially the same as the regular semantic versioning strategy but with the v prefix removed entirely to more closely follow the semantic versioning standard?

@webbertakken
Copy link
Member

Sorry, I'm not sure if I understand why users would have to count 'other commits' for versioning and why users would have to remove existing tags. All this change would introduce is the v prefix being optional or not.

Sorry that was a typo, I meant to include more tags in versioning. with other commits I meant additional tags. Edited.

About the rest of your arguments, see my last post. This already works exactly how you describe you expect it to work.

@abigailwillow
Copy link
Author

As discussed on Discord it seems there was a misunderstanding between what I believed the issue was and what @webbertakken believed the issue was. The v not being optional is not intended behavior and should be fixed soon. Additionally an update to the documentation may still be in place to clarify.

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

Successfully merging this pull request may close these issues.

3 participants