Skip to content

Add new release labels - #7499

Merged
leecow merged 16 commits into
mainfrom
release-labels
Oct 27, 2022
Merged

Add new release labels#7499
leecow merged 16 commits into
mainfrom
release-labels

Conversation

@richlander

@richlander richlander commented Jun 1, 2022

Copy link
Copy Markdown
Member

Apply new labels to release-index.json and release.json.

This PR will be merged on July 13th.

Context: dotnet/designs#265

We are also changing the support-phase property, which is a breaking change at GA time. The property will follow the progression below:

preview -> rc -> active -> maintenance -> eol

The new release-type property will be set as either lts or standard and will not change, even after EOL.

The spec suggests a string array of labels. On further discussion, that doesn't seem necessary. This simpler approach seems better and more efficient for readers. The only label we're missing is latest. It doesn't seem warranted to add another property to account for that.

I updated all the releases in release-index.json to make it internally consistent, but then only updated the in-support releases for release.json. That's definitely up for discussion. Seemed pragmatic.

@mairaw

@rbhanda

rbhanda commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

@mairaw can we test this for dot.net to ensure this doesn’t break it?

@richlander

Copy link
Copy Markdown
Member Author

We know that this change will break the website (mostly due to the same issue as #7497). @mairaw would like to test this change, as you suggest, to see if there are other issues.

If we have other known contacts who use these files, now would be a good time to contact them.

@mairaw

mairaw commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

The new property doesn't cause any issues on our ingestion process. And I just created a PR to not rely on eol-date being null to determine support phase, which can be merged today. With that change merged, we could even add the .NET 7 EOL dates back in.

@richlander

Copy link
Copy Markdown
Member Author

With that change merged, we could even add the .NET 7 EOL dates back in.

This PR does that. We can wait to merge this PR or unrevert the other one. Might as well wait and then we can tell people about the changes all at once.

@mairaw

mairaw commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

Yep, either way is fine.

@richlander

Copy link
Copy Markdown
Member Author

I'll leave that up to you. Whichever approach you prefer.

@joeloff

joeloff commented Jun 7, 2022

Copy link
Copy Markdown
Member

Are you planning on adding the release-type to older releases.json files, e.g. 1.0 or 2.2? If not, what should be the default value when reading these files for the property? We have use cases for processing these files and I want to make sure the releases API handles this in a consistent fashion. I could also consider an undefined enum.

@richlander

Copy link
Copy Markdown
Member Author

I updated all the release objects in release-index.json. I didn't update the old release.json files in absence of a reason to do so. If you have a scenario where that would be helpful, I'm willing to do it. It won't take me very long. Should I?

@joeloff

joeloff commented Jun 7, 2022

Copy link
Copy Markdown
Member

I updated all the release objects in release-index.json. I didn't update the old release.json files in absence of a reason to do so. If you have a scenario where that would be helpful, I'm willing to do it. It won't take me very long. Should I?

That would be great, unless you have a suggestion for assigning a default value when the release-type is absent in the JSON - I'd be open to that as well.

@mairaw

mairaw commented Jun 23, 2022

Copy link
Copy Markdown
Contributor

I'll start working on the PR for the website now. @richlander do you want to handle the merge conflicts here?

@richlander

Copy link
Copy Markdown
Member Author

Happy to deal with the merge conflicts. However, I think I promised that I wouldn't merge these files until 7/13 (or some similar date). I'd rather wait until July patch Tuesday. Fair?

@mairaw

mairaw commented Jun 23, 2022

Copy link
Copy Markdown
Contributor

Of course, no problem!

@mairaw

mairaw commented Jun 23, 2022

Copy link
Copy Markdown
Contributor

I just dealt with the merge conflicts since I also had to add the missing property to all other versions. And when would we add the changes to the support-phase property? I just need to account for the interim solution with the current values.

@richlander

Copy link
Copy Markdown
Member Author

Thank @mairaw!

Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json Outdated
@timheuer

Copy link
Copy Markdown

Do we have tests or any validation for install scripts? There are args that expect Channel that will now be changing (https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#options)

@richlander

richlander commented Jun 28, 2022

Copy link
Copy Markdown
Member Author

now changing

I assume you mean "not changing"

Current - Most current release.

Looks like that can still be satisfied w/o issue. I'm not sure if what it says is what it does. Good question.

Maybe @YuliiaKovalova and @dotnet/install-scripts-maintainers can tell us.

@timheuer

Copy link
Copy Markdown

Yes @richlander I'm thinking to make sure things like -Channel sts would be expected to work...I think it will work based on my 2m reading of the code, but might need to just expand test validation/coverage of adding the new labels.

@bekir-ozturk

Copy link
Copy Markdown

As I understand it, the label Current works as expected with install scripts (installs .NET 6.0) while it undesirably refers to 5.0 in VS.

@richlander already mentioned that this will be a breaking change, but I wanted to double check that we are on the same page about what exactly will be broken. So, here are some questions that popped into my head:

  1. Does removal of Current label also mean the removal of Current channel? @mmitche @adiaaida
  2. Current allows the users of install scripts to say "give me whatever is the latest GA version, whether it is LTS or not" which (warning: speculation ahead!) may be a common scenario for people wanting to stay up to date (end of speculation). If the channel Current will indeed be removed, can we say that "this is one of the breaking changes we are introducing"? What alternative can we offer for these users?
  3. As we are updating install scripts documentation, do we want to add sts to the list of supported channels? Agreeing with @richlander above, it may be an anti-pattern since from the context of the scripts it will mean "give me the latest GA build, but make sure it is one of those that will expire soon".
    I'm unsure whether the benefits of having an sts channel in scripts will worth the maintenance costs as well as the potential user confusion.
@michellemcdaniel

Copy link
Copy Markdown
  1. Does removal of Current label also mean the removal of Current channel? @mmitche @adiaaida

Current in terms of the concept that users can dotnet-install --version current (ie, the aka.ms links) seem like they would be unaffected by this change. Creating the current links is a manually run pipeline that we do once a release (more or less on GA release day).

I'm unsure whether the benefits of having an sts channel in scripts will worth the maintenance costs as well as the potential user confusion.

If we add sts to dotnet-install we will need to update a bunch of stuff in other places. Please ping me if we decide to do this.

@richlander

Copy link
Copy Markdown
Member Author

Let's just stay as-is. I will write up some docs and probably a blog post on our updated approach. I will review it with you folks to ensure everything is correct.

We could change "Current" to "Latest" but that seems like a lot of churn for little benefit. I am NOT proposing that.

Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
Comment thread release-notes/releases-index.json
@mairaw

mairaw commented Oct 20, 2022

Copy link
Copy Markdown
Contributor

@jamshedd I resolved all the comments because changing those is no longer the plan of record. Please review again.

@mairaw
mairaw requested a review from jamshedd October 20, 2022 23:40

@leecow leecow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good to go - we'll merge on release day.

Comment thread release-notes/1.0/releases.json
@mairaw

mairaw commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

These changes should be included with all other releases.json changes for GA. We won't be able to handle different changes coming at different PRs for the website (or we'll need to skip some deployments).

Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json Outdated
Comment thread releases-index.json Outdated
leecow and others added 6 commits October 27, 2022 11:15
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Comment thread release-notes/6.0/releases.json Outdated
"latest-runtime": "6.0.10",
"latest-sdk": "6.0.402",
"release-type" : "lts",
"support-phase": "lts",

@leecow leecow Oct 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mairaw - support-phase should be 'active'. Is it OK to make that change for the 'catch-up' merge today?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, I've been testing to what's in the private repo so those are the changes I'm expecting to see and I'm prepared to handle

update support phase for 6.0 to 'active'.
Comment thread release-notes/releases-index.json Outdated
Comment thread release-notes/releases-index.json Outdated
leecow and others added 2 commits October 27, 2022 15:54
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
@leecow
leecow merged commit 5d00f8b into main Oct 27, 2022
@leecow
leecow deleted the release-labels branch October 27, 2022 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

10 participants