Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Modify BuilderExtensions.UseMvc to not add any routes by default #1880

Closed
wants to merge 1 commit into from

Conversation

pranavkm
Copy link
Contributor

Fixes #1879

/// Adds Mvc to the <see cref="IApplicationBuilder"/> request execution pipeline.
/// </summary>
/// <param name="app">The <see cref="IApplicationBuilder"/> to add Mvc too.</param>
/// <returns>The <paramref name="app"/>.</returns>
public static IApplicationBuilder UseMvc([NotNull] this IApplicationBuilder app)
Copy link
Member

Choose a reason for hiding this comment

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

nit: should docs include <remarks/> describing expectation application uses attribute routes exclusively?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ehh, not super sure on this - remarks shouldn't be a place for specifying application design guidance. This sounds more like a sample \ doc thing

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we could say something here without it being design guidance.

UseMvc() only supports attribute routing. To add conventional routes use AddMvc(...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

@dougbu
Copy link
Member

dougbu commented Jan 23, 2015

:shipit: after minor fixes

/// <summary>
/// Adds Mvc to the <see cref="IApplicationBuilder"/> request execution pipeline.
/// </summary>
/// <param name="app">The <see cref="IApplicationBuilder"/> to add Mvc too.</param>
Copy link
Member

Choose a reason for hiding this comment

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

The to add Mvc too.

The <see cref="IApplicationBuilder"/>. IMO

@pranavkm
Copy link
Contributor Author

Updated

@@ -4,7 +4,6 @@
using Microsoft.AspNet.Builder;
using Microsoft.AspNet.Mvc;
using Microsoft.AspNet.Mvc.Xml;
using Microsoft.AspNet.Routing;
Copy link
Member

Choose a reason for hiding this comment

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

ah, so this'll go in after your Routing change?

@dougbu
Copy link
Member

dougbu commented Jan 24, 2015

:shipit:

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

Successfully merging this pull request may close these issues.

4 participants