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

Adding IView.Path and ViewContext.ExecutingPagePath #1996

Closed
wants to merge 1 commit into from
Closed

Conversation

pranavkm
Copy link
Contributor

Fixes #1940

@ghost ghost added the cla-not-required label Feb 11, 2015
@pranavkm
Copy link
Contributor Author

cc @DamianEdwards \ @dougbu

/// Asynchronously renders the view using the specified <paramref name="content"/>.
/// </summary>
/// <param name="context">The <see cref="ViewContext"/>.</param>
/// <returns>A <see cref="Task"/> rthat represents the asynchronous rendering.</returns>
Copy link
Member

Choose a reason for hiding this comment

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

  • typo: "that"
  • avoid "represents" since it's meaningless. suggest A <see cref="Task"/> that on completion renders the view.

@dougbu dougbu changed the title Adding IView.Path and ViewContext.ExecutingPagePatg Adding IView.Path and ViewContext.ExecutingPagePath Feb 11, 2015
@dougbu
Copy link
Member

dougbu commented Feb 11, 2015


I corrected the typo in the PR title. please do the same in your commit description 😄

@yishaigalatzer
Copy link
Contributor

:shipit: when @dougbu is happy with the comments

@pranavkm
Copy link
Contributor Author

Updated.

@dougbu
Copy link
Member

dougbu commented Feb 12, 2015

I'm reluctant about the lambda property for a read only value. if you can hold off pushing, let's talk about that tomorrow.

@dougbu
Copy link
Member

dougbu commented Feb 12, 2015

:shipit:

public interface IView
{
/// <summary>
/// Gets the path of the view as resolved by the <see cref="ViewResult"/>.
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this come from the view engine? there's nothing stopping you from using this outside of ViewResult

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

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