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

Rename AppendEncoded() to AppendHtml() #451

Closed
wants to merge 4 commits into from
Closed

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Oct 21, 2015

@dougbu
Copy link
Member Author

dougbu commented Oct 21, 2015

@kichalla

@@ -29,7 +29,7 @@ public interface IHtmlContentBuilder : IHtmlContent
/// </summary>
/// <param name="encoded">The HTML encoded <see cref="string"/> to append.</param>
/// <returns>The <see cref="IHtmlContentBuilder"/>.</returns>
IHtmlContentBuilder AppendEncoded(string encoded);
IHtmlContentBuilder AppendHtml(string encoded);
Copy link
Member

Choose a reason for hiding this comment

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

Should the documenation and parameter name change? example: AppendHtml("<b>") ...the string is not encoded but raw html...
Same comment in all other relevant places.

Copy link
Member Author

Choose a reason for hiding this comment

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

The parameter name is entirely correct and a good counterpoint to the unencoded parameter in Append(). In your example "<b>" is as encoded as it's going to get.

@dougbu dougbu force-pushed the dougbu/append.html.3225 branch from 607d975 to 8ef572c Compare October 22, 2015 17:03
Also correct parameter names and doc comments
- add `xml-docs-test` to ensure this doc comments remain valid in the future
@dougbu
Copy link
Member Author

dougbu commented Oct 22, 2015

@kichalla updated to fix parameter names, SetContentEncoded() name and doc comments.

@@ -26,9 +26,9 @@ internal class BufferedHtmlContent : IHtmlContentBuilder
/// </summary>
/// <param name="value">The <c>string</c> to be appended.</param>
Copy link
Member Author

Choose a reason for hiding this comment

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

xml-docs-test missed this. will fix.

@dougbu
Copy link
Member Author

dougbu commented Oct 22, 2015

@kichalla CI builds are failing because Coherence hasn't made recent Common changes available. This is ready to go.

@kichalla
Copy link
Member

:shipit:

@dougbu
Copy link
Member Author

dougbu commented Oct 22, 2015

bcb56bd

@dougbu dougbu closed this Oct 22, 2015
@dougbu dougbu deleted the dougbu/append.html.3225 branch October 22, 2015 23:46
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.

3 participants