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

Support Semantic Versioning 2.0.0 #29

Closed
wants to merge 2 commits into from

Conversation

Thilas
Copy link

@Thilas Thilas commented Jun 8, 2023

Now that chocolatey supports SemVer 2, we can also enable it in AU.

I also change a bit the parsing behavior to match chocolatey normalization of versions:

  • 1.2 => 1.2.0
  • 1.2.3.0 => 1.2.3

Last thing, I try to "improve" versions so that a SemVer 1 prerelease becomes SemVer 2

  • 1.2.3-rc4 => 1.2.3-rc.4

Pester tests has been changed accordingly.

@JPRuskin
Copy link

That's awesome! I'll try and have a look at it later today.

On the face of it, I am unsure about "improving" versions - should we not allow folk to use SemVer1 if they prefer? Is 'rc4' not a valid prerelease version even without the build number / dot-digit?

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphens [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes. Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version. Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92, 1.0.0-x-y-z.--.

@Thilas
Copy link
Author

Thilas commented Jun 13, 2023

You are correct, rc4 is a valid prerelease in SemVer2. Though, I would prefer rc.4 notation for my packages because it won't break version order if it comes to rc.10. That's the whole reason why I introduced this behavior.

An other way would be to introduce an option to let maintainers decide what behavior they want for their packages. For instance, we could add a Get-Version2 cmdlet (or a new parameter: Get-Version -Enhanced). Either way, it would return an [AUVersion] instance.

@TheCakeIsNaOH
Copy link
Member

An other way would be to introduce an option to let maintainers decide what behavior they want for their packages. For instance, we could add a Get-Version2 cmdlet (or a new parameter: Get-Version -Enhanced). Either way, it would return an [AUVersion] instance.

I think this is a better option than requiring the normalization. Ideally, AU would have 1 to 1 support with the versioning syntax that Chocolatey CLI supports. Then if maintainers want to automatically fix/improve the version in use, they have the option, and also this would not be a breaking change (I think).

@Thilas
Copy link
Author

Thilas commented Jun 17, 2023

I'm gonna push soon my new changes to have such an option.

In parallel, it happens that I had my 1st dotted prerelease package and was unable to push it: chocolatey/home#264

Probably worth waiting a little bit longer before changing AU.

@TheCakeIsNaOH
Copy link
Member

Probably worth waiting a little bit longer before changing AU.

It may still be worthwhile, as AU can be used with internal repositories that do support SemVer v2

@Thilas
Copy link
Author

Thilas commented Jun 17, 2023

Since Chocolatey Community Repository does not support SemVer2 right now, then either we leave AU as is for the time being. Or we can introduce 3 different behaviors for Get-Version:

  • SemVer1 (default)
  • SemVer2 (to use with compatible repo only)
  • EnhancedSemVer2 (as discussed above)

Any thoughts?

@Thilas
Copy link
Author

Thilas commented Jun 17, 2023

@TheCakeIsNaOH, seems like you were faster than me 😄

@Thilas
Copy link
Author

Thilas commented Jun 17, 2023

Here we go. I added an optional param -SemVer to Get-Version:

  • V1 (default) is basically the current behavior
  • V2 is the new behavior when SemVer2 is supported by the destination gallery
  • EnhancedV2 is this new mode that make SemVer1 versions looks better

I completely reviewed the unit tests for AUVersion class and Get-Version cmdlet, especially to show the differences between the different modes.

@Thilas Thilas force-pushed the master branch 2 times, most recently from 97bb393 to 2a21db0 Compare June 19, 2023 13:27
@corbob
Copy link
Contributor

corbob commented Mar 21, 2024

@Thilas Thank you for this PR. Last week we released the first release of the module under the Chocolatey Community group under the name Chocolatey AU. As part of the release, we have moved the repository over to a GitFlow layout. Could you rebase your branch on chocolatey-community/develop and then point this PR at the develop branch?

It is possible that the rebase will have some issues, If you'd like some assistance with getting it pointed at develop, feel free to ping me and I can assist with the commands needed to complete the rebase.

Could you also update the description to include a Testing section to outline what testing was done? Bonus points if you're able to add a Pester test that will test this issue.

@Thilas
Copy link
Author

Thilas commented Mar 22, 2024

@corbob, done

Regarding tests, this PR is 9 months old. I have been using it on my own packages for that long without any issue + I don't remember what tests I did exactly back then. Though existing Pester tests were completely rewrote both to adjust to semver2 but also to better cover the different use cases.

@Thilas Thilas changed the base branch from master to develop March 22, 2024 07:51
@Thilas
Copy link
Author

Thilas commented Mar 22, 2024

Looks like some of the pester tests are failing. I'll keep you posted.

@Thilas Thilas force-pushed the master branch 2 times, most recently from 4af5cf9 to 6a26e9b Compare March 24, 2024 13:35
@Thilas
Copy link
Author

Thilas commented Mar 24, 2024

@corbob, tests are now OK.

@pauby pauby closed this May 14, 2024
@Thilas
Copy link
Author

Thilas commented May 14, 2024

@pauby, care to explain why you are closing (this PR as well as others) without any comment?

@pauby
Copy link
Member

pauby commented May 14, 2024

@Thilas I wasn't aware I did close this PR and others. Which is concerning.

Just a point to note on the tone of your comment - it doesn't come across as you intended.

@pauby pauby reopened this May 14, 2024
@pauby pauby closed this May 14, 2024
@pauby pauby reopened this May 14, 2024
@pauby pauby closed this May 14, 2024
@pauby
Copy link
Member

pauby commented May 14, 2024

Tried reopening this PR, and it immediately closes it again. So I'm at a loss.

Looking at the timings, the repository was detached from its parent just before this PR was closed. That doesn't feel related, as it happened 23 minutes later. But it could be.

Unless anybody has an idea how to resolve this, I'm going to suggest submitting the PR again. However, please don't submit it from your master branch. Please create a separate branch for it.

PR 30 and 31 (I'm not linking them here) are also affected. All of them are your PR's. Did you fork chocolatey-au or did you use the original au repository for your PR's? That is the only thing I can think of. As the repository was detached from the original au repository, it may have closed them automatically. If that it is the case, as I requested the detachment, this is being done under my account.

@pauby pauby reopened this May 14, 2024
@pauby pauby closed this May 14, 2024
@Thilas
Copy link
Author

Thilas commented May 14, 2024

Just a point to note on the tone of your comment - it doesn't come across as you intended.

My bad, I was surprised but didn't want to sound rude.

@pauby pauby reopened this May 14, 2024
@pauby pauby closed this May 14, 2024
@pauby
Copy link
Member

pauby commented May 14, 2024

I was surprised

You and me both. Still unsure of what happened.

@corbob corbob reopened this May 14, 2024
@corbob corbob closed this May 14, 2024
@corbob
Copy link
Contributor

corbob commented May 14, 2024

I was thinking maybe it was because @pauby was the one that had the repository changed. But apparently it auto-closes even when I try to reopen it 😬

@pauby
Copy link
Member

pauby commented May 15, 2024

Thanks for trying @corbob.

@pauby
Copy link
Member

pauby commented May 16, 2024

@Thilas I believe we have solved this problem.

Your fork is from the original AU. I'm going to assume that when we detached from that upstream, it closed these PR's because of that. This will be the reason for #30 and #31 also being closed.

Can you fork this repository and create PR's from there?

image

@Thilas
Copy link
Author

Thilas commented May 16, 2024

I see, I'll do that later then. Thank you!

@Thilas Thilas mentioned this pull request May 23, 2024
10 tasks
@Thilas
Copy link
Author

Thilas commented May 23, 2024

@pauby, I finally got some time to open new PR from a new fork and it works as expected. I'm going to do the same for my 2 other pull requests. Thanks.

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

Successfully merging this pull request may close these issues.

5 participants