Skip to content

Conversation

@Dreynor87
Copy link
Contributor

@Dreynor87 Dreynor87 commented Oct 27, 2025

Nuget will now follow:
M.m.p<-exp>+DateRev

A microsoft employee must use /azp run to validate using the pipelines below.

WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.

For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.

@Dreynor87
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@codendone codendone left a comment

Choose a reason for hiding this comment

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

See my comment. Also, is this intended as a temporary change or long-term change?

if(-not [String]::IsNullOrEmpty($versionTag))
{
$formattedTag = '-' + $versionTag
$formattedTag = '-' + $versionTag + "-" + $(versionMinDate) + $paddedRevision
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not SemVer compliant.

Copy link
Contributor Author

@Dreynor87 Dreynor87 Oct 29, 2025

Choose a reason for hiding this comment

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

Including the Date and Revision for the release one, or in all cases? Re-reading semver.org, if I change that to a "+" does that work? I switched because I thought that M.m.p+DateRev was not ok for the vpack code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, switching to "+" should address the SemVer issue. I agree that vpack limitations also need to be considered (I'm not familiar enough there to know whether there is a problem).

I'm still curious if this is a temporary change or intended long-term. If the latter, is there a spec to review?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do intend this to be long term. I am adding the date and revision in case we need to rebuild something internal (as we have seen multiple times trying to get 2.0.0-exp2 out the door). We dont want to have to bump everything another patch because of a problem with a single metapackage. I have not put this recent-ish design change into a spec yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not excited about this as the long-term design. I strongly recommend a spec and reviewing that before implementing something across all components. Differences from the old 2.0 spec would be especially good to discuss.

Copy link
Member

Choose a reason for hiding this comment

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

+stuff = build info, which is entirely informational. Whether and how this should be valued is hard to say given the lack of spec. We need spec, before doing any implementation.

RECOMMEND: Create specs/SemanticVersion.md and spell out how semver is defined/used (and of course follow up with spec review :-)

FYI per https://semver.org

<valid semver> ::= <version core>
                 | <version core> "-" <pre-release>
                 | <version core> "+" <build>
                 | <version core> "-" <pre-release> "+" <build>

@Dreynor87 Dreynor87 self-assigned this Oct 30, 2025
@Dreynor87 Dreynor87 requested a review from kythant October 30, 2025 16:17
@Dreynor87
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants