Skip to content

[Route Groups] Rename GroupRouteBuilder to RouteGroupBuilder #41781

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

Closed
Tracked by #41433
halter73 opened this issue May 21, 2022 · 2 comments · Fixed by #41822
Closed
Tracked by #41433

[Route Groups] Rename GroupRouteBuilder to RouteGroupBuilder #41781

halter73 opened this issue May 21, 2022 · 2 comments · Fixed by #41822
Labels
api-approved API was approved in API review, it can be implemented old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Milestone

Comments

@halter73
Copy link
Member

halter73 commented May 21, 2022

Background and Motivation

RouteGroupBuilder sounds better than GroupRouteBuilder. In the original API proposal (#36007) this was called RouteGroup which is closer to the name I am suggesting here. RouteGroup is also an alternative name that I think is better than GroupRouteBuilder. I proposed GroupRouteBuilder in the follow up comments because it better aligned with IEndpointRouteBuilder, but now I think that's unimportant.

Proposed API

namespace Microsoft.AspNetCore.Routing;

- public sealed class GroupRouteBuilder : IEndpointRouteBuilder, IEndpointConventionBuilder
+ public sealed class RouteGroupBuilder : IEndpointRouteBuilder, IEndpointConventionBuilder

Usage Examples

Before

GroupRouteBuilder group = app.MapGroup("/todos");
group.MapGet("/", (int id, TodoDb db) => db.ToListAsync());

After

RouteGroupBuilder group = app.MapGroup("/todos");
group.MapGet("/", (int id, TodoDb db) => db.ToListAsync());

Alternative Designs

  • Keep it as GroupRouteBuilder since that puts "Group" up front even if using "Group" as an adjective sound weird.
  • Go with the ven shorter RouteGroup.

Risks

Low. It's only been released in preview4 and it would only break apps that reference the type name instead of just using var.

@halter73 halter73 added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label May 21, 2022
@ghost
Copy link

ghost commented May 21, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@Pilchie Pilchie added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 22, 2022
@javiercn javiercn added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 23, 2022
@halter73
Copy link
Member Author

API review notes:

  1. Should we do this?
    This has only been out for a preview release. A lot of people use var. Now or never, and it seems slightly better.

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels May 23, 2022
@BrennanConroy BrennanConroy added this to the 7.0-preview5 milestone May 24, 2022
@halter73 halter73 changed the title Rename GroupRouteBuilder to RouteGroupBuilder [Route Groups] Rename GroupRouteBuilder to RouteGroupBuilder May 25, 2022
@halter73 halter73 mentioned this issue May 25, 2022
4 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Jun 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants