-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[Pre ?] Remove comment about Minimal API's not supporting ProducesResponseType #35752
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks @sander1095 ... The way that you were proceeding isn't the way that we roll out our preview coverage. For one thing, we usually don't explicitly indicate the preview that a feature rolled out in. We simply document the features as they come out in the framework. If we need to provide some additional context about the feature, for example, further enhancements, we just call them out. Later, we revise the statements as the feature evolves. Therefore, I think the best way to proceed ... if I'm understanding the state of affairs here ... is to go ahead and make these changes with the remark stating that Minimal APIs aren't supported at this time. Then ... later ... after your PU PR merges and we know which preview it will appear in, the remark can be pulled on a new PR that will fix your other issue at #35753. Am I getting the gist of where your work stands and how we can proceed? Let me know if I'm not understanding some aspect(s) of this. π |
Hi @guardrex . We are on completely different wavelenghts here, and I would request that you undo the (non-formatting) changes you made. Perhaps you could have waited with these changes until I got back to you. (I don't mean this in an annoyed way :) ) The way I see it:
My original changes in this PR never referenced a preview version, so I don't know what you mean with this. My changes in this PR were in line with this policy, and the other issue I created was in line with the release notes that are written for each preview version. If I understand what you mean, is that you wouldn't want the release notes of preview 7 (for example) to say that the bug has been fixed and the feature is now fully usable for Minimal API's. I understand that each bug fix doesn't need to be mentioned in a preview, but because the docs have said that Minimal API's don't support ProducesResponseType, and they now will, I think it's worth mentioning in the release notes. |
The concern for us is this bit ...
We don't know when your PU PR will be merged ...
Indeed so! π In the meantime, a backlog of PRs here waiting to be merged at some unknown future time isn't the best workflow for us. Your PR might not get merged over there until RC1 (September) ... or RC2 (October). We've found ourselves at times with a long list of open PRs here, leading in some cases to nasty merge conflicts π. Our best workflow is to know that your PU (product unit) PR has been merged, know what preview it will land in, and then have the docs PR prepared for that preview release. Since you think that they might merge it quickly and that it might make it for Preview 7, let's try it your way and see how it goes π€π. I'll revert the Minimal API line (delete it) and mark this PR with a typical title marker that I use when I don't know which preview something will land in ("[Pre ?]"). I'll keep an eye on your PU PR and mark this for Preview 7 if that's when it can go in. WRT the other issue that you opened and your comments about ...
We only keep one article for the 10.0 release at ... https://learn.microsoft.com/aspnet/core/release-notes/aspnetcore-10.0 We update it as we go each preview release. We don't have separate articles for each preview release here. Can you provide link(s) to the set of individual preview release note articles that you're referring to? UPDATE: Yes, I see from your other issue's remarks ...
... that you're saying that there are individual preview articles somewhere. Please provide the link(s). I'd like to understand what set of articles you're referring to. That's why I made the remark that confused you ...
|
Hi @guardrex! Apologies for my earlier tone, I was in a bit of a sour mood and it shouldn't have reflected in my comment. I completely understand your point about not wanting things to stay open too long. The articles I mean:
and more: https://devblogs.microsoft.com/dotnet/dotnet-10-preview-5/ (see other previews, too) |
No worries. I think this will be fine. Hopefully, they'll get your PR merged for Preview 7. Oh, yes ... I see what you mean. I don't think that they publish those on the Learn website, but I might be wrong. I'm more familiar with what they publish here from the .NET Core docs repo ... https://learn.microsoft.com/dotnet/core/whats-new/dotnet-10/overview We don't maintain the markdown files that you linked here. This repo only controls the ASP.NET Core articles. You should close your other issue here and open a new issue for them at ... https://github.com/dotnet/core/issues James Montemagno is managing those preview release markdown files. I'm not sure if he's just looking at our article and creating them or not. You could ask in the issue if what appears in our article is enough to get the feature covered over there when the time comes to add it. If so, then he won't need a separate issue on the .NET Core docs repo. Your #35751 issue here and this PR is all that we'll need to get coverage in both places. Sorry that it's so confusing what goes where and who manages what π©. We're rather highly compartmentalized, and I'm just a small green dinosaur π¦ in a BIG Microsoft world ππ. P.S. ... I have a suspicion that the Dev Blogs are automated. You can ask James if that's correct. |
Actually, let's see if he'll come on here for a sec ...... @jamesmontemagno ... TL;DR ... For your individual preview markdown files (e.g., https://github.com/dotnet/core/blob/main/release-notes/10.0/preview/preview5/aspnetcore.md), are you obtaining your ASP.NET Core information from our ASP.NET Core release notes article? If so, it seems like you don't need community devs to open separate issues on the .NET Core repo to get release notes coverage for preview features that they contribute. Is that correct? Follow-up β ... Are the Dev Blogs articles for preview releases (e.g., https://devblogs.microsoft.com/dotnet/dotnet-10-preview-5/) automated? I seem to recall that that's the case. If so, no issue is required to get a community-contributed framework feature covered there. |
Probably a @danroth27 question on release notes |
@danroth27 ... TL'DR ... Is the feature coverage in the individual preview markdown files (e.g., https://github.com/dotnet/core/blob/main/release-notes/10.0/preview/preview5/aspnetcore.md), obtained from our ASP.NET Core release notes? If so, it seems like you don't need a community dev to open an issue on the .NET Core repo to get coverage there for a feature they've contributed. Follow-up β ... Are the Dev Blogs articles for preview releases (e.g., https://devblogs.microsoft.com/dotnet/dotnet-10-preview-5/) automated? |
No, I source the information for the preview release notes from what the devs post on the corresponding What's New update issue in the aspnetcore.docs repo. I also scan the merged PRs for the milestone. The preview release notes are different from the What's New docs in that they need to document the preview-to-preview changes, while the What's New docs document the stable .NET version-to-version changes. @sander1095 We don't typically document bug fixes in the release notes because there can be a lot of them. Instead, we focus on new functionality that we want users to try out. It's assumed that important bugs will get fixed. We do also list the community contributors for each release and link to their merged PRs, so this fix should show up there.
I believe they are to a large extent. These blog posts mostly list highlights from the release notes. |
@danroth27 Alright! Thanks for the clear answer. @guardrex I believe this PR can still be merged. A few hours ago my code fix PR got merged for preview 7, which this PR can also be a part of. |
[EDIT by guardrex ... π HOLD π for PU PR merge and determination of which preview the changes will land in. I'll keep an ποΈ on this. βπ¦]
This PR removes the comment and fixes an incorrect reference to a community contributor.
Fixes #35751