Skip to content
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

Move MapOpenApi() Inside the IsDevelopment() block for consistency wi… #34541

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

davepcallan
Copy link

@davepcallan davepcallan commented Jan 23, 2025

…th templates and other docs

The default template with .NET 9 has MapOpenApi() inside the IsDevelopment block. Many examples on MS Learn also have it inside due to limit information exposure.

Move inside to minimize potential for confusion to devs.


Internal previews

📄 File 🔗 Preview link
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md aspnetcore/fundamentals/openapi/aspnetcore-openapi

davepcallan and others added 2 commits January 23, 2025 11:22
…th templates and other docs

The default template with .NET 9 has MapOpenApi() inside the IsDevelopment block. Many examples on MS Learn also have it inside due to limit information exposure. 

Move inside to minimize potential for confusion to devs.
@wadepickett
Copy link
Contributor

wadepickett commented Jan 23, 2025

@davepcallan, thanks for calling this out and providing an example update! See and address Contributor License Agreement request above before we can continue.

I will update for the rest of the code that is sectioned with preprocessor directives (only the #Default sample originally used the Develpment environment condition, likely for simplicity's sake in the doc examples). I will also verify the documents referring to the code still display correctly with the changes.

@davepcallan
Copy link
Author

@dotnet-policy-service agree

@@ -3,7 +3,6 @@
//#define DOCUMENTtransformer1
//#define DOCUMENTtransformer2
#define DOCUMENTtransformerUse999
Copy link
Contributor

Choose a reason for hiding this comment

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

Removed dupe preprocessor directive for default.

@wadepickett wadepickett self-assigned this Jan 23, 2025
@@ -80,7 +79,10 @@ internal record WeatherForecast(DateTime Date, int TemperatureC, string? Summary

var app = builder.Build();

app.MapOpenApi();
if (app.Environment.IsDevelopment())
Copy link
Contributor

Choose a reason for hiding this comment

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

Added Development environment condition for all calls to app.MapOpenAPI() and tested each.

Line 501, moved app.MapOpenAPI() to same environment condition block for app.MapScalarApiReference()
@wadepickett
Copy link
Contributor

wadepickett commented Jan 23, 2025

Some code highlighting is off a few lines as a result in topics that refer to the code. Three topics use the code inline. Only one had a highlighting alignment issue. Updating that...

@wadepickett
Copy link
Contributor

wadepickett commented Jan 23, 2025

@captainsafia, is this change to move app.MapOpenApi() within the condition of isDevelopment OK with you? The #defualt code had one but the other examples did not. I also reviewed topics referring to the sample and updated a code highlight. Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants