Middleware/Rewrite: Update XML comments for "AddRedirect" to indicate status code. #57224
Conversation
…t status code it returns.
dotnet-issue-labeler
bot
added
the
area-middleware
dotnet-policy-service
bot
added
area-networking
| @@ -76,7 +76,7 @@ public static RewriteOptions AddRedirect(this RewriteOptions options, [StringSyn | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Redirect a request to https if the incoming request is http, with returning a 301 | |||
| /// Redirect a request to https if the incoming request is http, with returning a 301 - Moved Permanently status code. | |||
| /// status code for permanently redirected. | |||
There was a problem hiding this comment.
status code for permanently redirected.
This one already says it sets 301 meaning permanently redirected.
There was a problem hiding this comment.
Yes, Also then tweaked the comment for the AddRedirectToHttpsPermanent method to make it consistent.. This is the only other overload in this class which mentions a status code, so it felt like the two should be aligned.
I can revert the change to this comment, and alter the comment for AddRedirect to be simply ,with returning a 302 (without the - Found status code), which would then make it consistent with the AddRedirectToHttpsPermanent comments if that is preferable.
There was a problem hiding this comment.
Changing the format is fine, but you kept part of the old comment.
There was a problem hiding this comment.
Yes, that is intentional? It just adds the text version of what the status code represents, in addition to the numeric status code.
There was a problem hiding this comment.
Ok, if it's intentional then status -> Status.
Bit I think you should just remove status code for permanently redirected. now.
There was a problem hiding this comment.
Oh wow, I completely missed that the xml comment for the permanent option was carried on to the next line, my mistake - no wonder you were confused about what I was doing. I have reverted the change to the permanent redirect option, and now aligned the 302 option to match what the permanent one does.
dotnet-policy-service
bot
added
the
pending-ci-rerun
* Align 302 coments to match.
|
Thanks @ctolkien |
Tweak XML comments for AddRedirect extension methods
Update XML comments to indicate the status code that is returned with this overload.
Description
Rationale here is I have just been burnt from thinking our site had permanent redirects in place, when they were actually temporary. There is no visibility into what status code is being used with this overload, unless you find it in the documentation. This just exposes the status code in the code comments.
Also then tweaked the comment for theAddRedirectToHttpsPermanentmethod to make it consistent.(Did not create a ticket first for this, as it didn't seem to fit the format of bug/feature)