Skip to content

update code sample in README#102

Merged
remarkablemark merged 1 commit intoremarkablemark:masterfrom
scott-silver:silver/update-readme
Apr 25, 2019
Merged

update code sample in README#102
remarkablemark merged 1 commit intoremarkablemark:masterfrom
scott-silver:silver/update-readme

Conversation

@scott-silver
Copy link
Copy Markdown
Contributor

What is the motivation for this pull request?

Better documentation

What is the current behavior?

Currently, the code sample in the README makes reference to a variable called parserOptions, even though that variable does not exist within the sample.

It seems that this variable is actually a reference to the object that contains the replace method, and that object should get passed in as the second argument to the calls to domToReact.

What is the new behavior?

The new code sample creates a constant called parserOptions, which can then be passed as the second argument to domToReact.

Thanks!

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 24, 2019

Coverage Status

Coverage remained the same at 99.296% when pulling f3f524e on scott-silver:silver/update-readme into 1d4e760 on remarkablemark:master.

Copy link
Copy Markdown
Owner

@remarkablemark remarkablemark left a comment

Choose a reason for hiding this comment

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

Looks good @scott-silver! Do you mind amending your commit message so it follows the conventional commits format?

$ git commit --amend -m "docs(readme): update code sample"
$ git push origin silver/update-readme -f

It helps with determining the semver release, which is why CI lints the commit message.

</span>
</p>
`,
{
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Technically, a variable doesn't need to be created here. However, I am okay with this refactor to improve readability.

README.md Outdated
@@ -125,6 +125,26 @@ import { renderToStaticMarkup } from 'react-dom/server';
import parse from 'html-react-parser';
import domToReact from 'html-react-parser/lib/dom-to-react';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since we're updating this code block, do you mind remove this line and updating the line above?

import parse, { domToReact } from 'html-react-parser';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, updated

@scott-silver scott-silver force-pushed the silver/update-readme branch from fb0d9ad to b34afd8 Compare April 25, 2019 20:56
@scott-silver scott-silver force-pushed the silver/update-readme branch from b34afd8 to f3f524e Compare April 25, 2019 20:57
@remarkablemark
Copy link
Copy Markdown
Owner

The build failure was due to an error in the commitlint script. It's been resolved in 4453dda

@remarkablemark remarkablemark merged commit e43da96 into remarkablemark:master Apr 25, 2019
@remarkablemark
Copy link
Copy Markdown
Owner

@scott-silver This will go out in the next release when a feature/bugfix has been merged

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.

3 participants