Skip to content

Conversation

@mHulb
Copy link
Contributor

@mHulb mHulb commented May 31, 2017

Hello everyone!
After working through this chapter of the documentation I propose to expand the code-snippet of the validation function for the SandwichOrder's FormBuilder.
In the style of the other examples, there should be more surrounding code to make it easier for inexperienced readers to understand where the validation function should be added to. During my first read-through, I had to pull up and search the corresponding sample project to understand where this code should live, which was different from other parts of the documentation previously.

Hello everyone!
After working through this chapter of the documentation I propose to expand the code-snippet of the validation function for the SandwichOrder's FormBuilder.
In the style of the other examples, there should be more surrounding code to make it easier for inexperienced readers to understand where the validation function should be added to. During my first read-through, I had to pull up and search the corresponding sample project to understand where this code should live, which was different than other parts of the documentation previously.
@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@RobStand
Copy link
Contributor

Hi @mHulb! Thanks for your contribution. The author of this snippet will review your PR. Thanks!

@RobStand RobStand requested a review from kbrandl May 31, 2017 17:50
Copy link
Contributor

@kbrandl kbrandl left a comment

Choose a reason for hiding this comment

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

Thank you for contributing this documentation update! I've added comments to the PR to request a few minor changes; if you can please update your snippet accordingly we'll merge those changes in to the live docs. Thanks again!

...
return new FormBuilder<SandwichOrder>()
.Message("Welcome to the sandwich order bot!")
...
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the "..." here and instead add these four lines that precede the Toppings line in the sample app:

.Field(nameof(Sandwich))
.Field(nameof(Length))
.Field(nameof(Bread))
.Field(nameof(Cheese))

select topping).ToList();
}
return result;
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Please indent "})" further so that its indentation matches the opening brace "{" for the validate function.

}
return result;
})
.Message("For sandwich toppings you have selected {Toppings}.")
Copy link
Contributor

Choose a reason for hiding this comment

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

After the line .Message("For sandwich toppings you have selected {Toppings}."), please add these 3 lines:

        ...
        .Build();
}

Four your reference (re proper indentation), here's what all four of those lines should look like together:

        .Message("For sandwich toppings you have selected {Toppings}.")
        ...
        .Build();
}

return result;
})
.Message("For sandwich toppings you have selected {Toppings}.")
.Message("For sandwich toppings you have selected {Toppings}.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Finally, please remove the duplicate line .Message("For sandwich toppings you have selected {Toppings}.") from the end of the snippet.

Hello @kbrandl! Thank you for the opportunity to contribute to the documentation. I've applied the requested changes to the snippet.
Copy link
Contributor

@kbrandl kbrandl left a comment

Choose a reason for hiding this comment

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

@mHulb, thank you for the quick response! Changes look great. @RobStand, I approve this PR.

@RobStand RobStand merged commit 3a44116 into MicrosoftDocs:master May 31, 2017
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.

4 participants