Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update readme.md #26

Closed
wants to merge 1 commit into from
Closed

Update readme.md #26

wants to merge 1 commit into from

Conversation

teinett
Copy link
Contributor

@teinett teinett commented Jul 2, 2021

Added installation of package unist-builder.
The example in readme.md file doesn't work without it.

@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Jul 2, 2021
@github-actions
Copy link

github-actions bot commented Jul 2, 2021

Hi! It seems you removed the template which we require. Here are our templates (pick the one you want to use and click *raw* to see its source):

I won’t send you any further notifications about this, but I’ll keep on updating this comment, and hide it when done!

Thanks,
— bb

Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

unist-builder is not required to use the module.
It is shown in the example, but unist-util-visit works fine independently.

A potential ideas to help clarify this:

  • add a "unist-builder is used here" type note to the use section
  • or update the example to use plain JSON, rather than a builder
  • or something else

Thoughts? 💭

@ChristianMurphy ChristianMurphy added the 📚 area/docs This affects documentation label Jul 2, 2021
@teinett
Copy link
Contributor Author

teinett commented Jul 2, 2021

I tried to use the code of example without npm install unist-builder. The example gives the error:

ast

@wooorm
Copy link
Member

wooorm commented Jul 2, 2021

Could you answer @ChristianMurphy’s comment?

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Jul 2, 2021

I tried to use the code of example without npm install unist-builder. The example gives the error ...

Right, "install" shows how to install this module, "use" provides an example of this module used along side another module.

For example, consider if the example was replaced with

import {visit} from 'unist-util-visit'
import remark from 'remark'

const tree = remark().parse('Some _emphasis_, **importance**, and `code`.')

visit(tree, (node) => console.log(node))

this shows usage with remark, without unist-builder.
unist-util-visit is the key part here, not remark or unist-builder, encouraging installing a dependency which is not needed could cause confusion and/or add an unnecessary dependency.

@teinett
Copy link
Contributor Author

teinett commented Jul 2, 2021

For beginners, it would be helpful to have a readme part "Don't forget" with text, how to use the example from zero

Don't forget to install all dependencies for this example: `npm install unist-builder unist-util-visit`

@ChristianMurphy
Copy link
Member

For beginners, it would be helpful to have a readme part "Don't forget" with text, how to use the example from zero

I agree, that's one of the suggestions from #26 (review)
Would you be interested in updating the readme to include that, or one of the other suggestions?

@teinett
Copy link
Contributor Author

teinett commented Jul 2, 2021

Would you be interested in updating the readme to include that, or one of the other suggestions?
I didn't know how to change this pull request, so I created the new one: #27.

If it's OK for you, I can update readme files of other projects with the same pattern.

@teinett
Copy link
Contributor Author

teinett commented Jul 2, 2021

Changed in #27

@teinett teinett closed this Jul 2, 2021
@teinett teinett deleted the patch-1 branch July 2, 2021 16:35
@github-actions
Copy link

github-actions bot commented Jul 2, 2021

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.

@wooorm wooorm added the 🤷 no/invalid This cannot be acted upon label Jul 2, 2021
@wooorm wooorm added 👎 phase/no Post cannot or will not be acted on and removed 👋 phase/new Post is being triaged automatically labels Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 area/docs This affects documentation 🤷 no/invalid This cannot be acted upon 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging this pull request may close these issues.

3 participants