Skip to content

Conversation

@heaths
Copy link
Member

@heaths heaths commented Mar 10, 2021

Fixes #1389

@heaths heaths requested a review from a team as a code owner March 10, 2021 03:04
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

#>

class AzureEngSemanticVersion {
class AzureEngSemanticVersion : IComparable {
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this I've been wanting to do that for a while.

cc @danieljurek as I know he was also looking at potentially doing this.

Copy link
Member

Choose a reason for hiding this comment

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

This is a much better (more readable) version than the one I wrote. I vote we use this one. Did some local sanity checking and this implementation checks out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially I tried implementing the generic IComparable[AzureEngSemanticVersion] interface, but seems you can't do that in PowerShell given the order of when types are resolved vs. compiled.

return $versions | Sort-Object -Descending
}

static [void] QuickTests()
Copy link
Member

Choose a reason for hiding this comment

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

If you haven't already could you please do a quick run of this to make sure the basic tests we have still work the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did, yes. In fact, I had to to debug an issue I was having in the original iteration.

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @heaths.

@ghost
Copy link

ghost commented Mar 10, 2021

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit f1c5a2a into Azure:master Mar 10, 2021
@heaths heaths deleted the issue1389 branch March 10, 2021 19:59
@heaths
Copy link
Member Author

heaths commented Mar 10, 2021

@weshaggard @chidozieononiwu @danieljurek should I send out an email about this, and make sure people know to add (preferably accurate) dates for older versions in their changelogs?

@weshaggard
Copy link
Member

I don't think it is worth sending an email as most folks will ignore it. If there are any issues they will come to light in the version increment PRs.

This pull request was closed.
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.

Release script should sort by date first, version second

4 participants