-
Notifications
You must be signed in to change notification settings - Fork 5k
Add new release labels #7499
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
Add new release labels #7499
Conversation
|
@mairaw can we test this for dot.net to ensure this doesn’t break it? |
|
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. |
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. |
|
Yep, either way is fine. |
|
I'll leave that up to you. Whichever approach you prefer. |
|
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. |
|
I updated all the release objects in |
That would be great, unless you have a suggestion for assigning a default value when the |
|
I'll start working on the PR for the website now. @richlander do you want to handle the merge conflicts here? |
|
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? |
|
Of course, no problem! |
|
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. |
|
Thank @mairaw! |
|
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) |
I assume you mean "not changing"
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. |
|
Yes @richlander I'm thinking to make sure things like |
|
As I understand it, the label @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:
|
If we add |
|
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. |
|
@jamshedd I resolved all the comments because changing those is no longer the plan of record. Please review again. |
leecow
left a comment
There was a problem hiding this comment.
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.
|
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). |
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
release-notes/6.0/releases.json
Outdated
| "latest-runtime": "6.0.10", | ||
| "latest-sdk": "6.0.402", | ||
| "release-type" : "lts", | ||
| "support-phase": "lts", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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'.
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Apply new labels to
release-index.jsonandrelease.json.This PR will be merged on July 13th.
Context: dotnet/designs#265
We are also changing the
support-phaseproperty, which is a breaking change at GA time. The property will follow the progression below:preview->rc->active->maintenance->eolThe new
release-typeproperty will be set as eitherltsorstandardand 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.jsonto make it internally consistent, but then only updated the in-support releases forrelease.json. That's definitely up for discussion. Seemed pragmatic.@mairaw