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

A couple of small things #85

Merged
merged 2 commits into from
Oct 26, 2018
Merged

A couple of small things #85

merged 2 commits into from
Oct 26, 2018

Conversation

danburzo
Copy link
Contributor

I've tried installing fathom on macOS / node 11.0, and I've gotten a variety of errors and warnings.

In this PR:

  • I've run npm audit fix --force to update the vulnerable dependencies (mocha and coveralls¹)
  • Added a link to the documentation site in the README. (It would be great if the URL in the GitHub page also pointed to the docs page rather than the npm package²)

I'm still getting an error for fsevents (via chokidar via babel-cli), but I think a fix requires upgrading to Babel 7, so it's not included in this PR.


¹ Hopefully it doesn't break the CI.
² I could include a short Installation section in the readme with the npm package name to supplement the changed URL.

@erikrose
Copy link
Contributor

Looks like it didn't break the CI! Thanks for the updates!

Of course, the link I had in the readme already linked to the docs, though a specific part. But, since it was obviously unclear to you, I gratefully accept your reformulation. :-) Thanks again!

@erikrose erikrose merged commit 5abfa3f into mozilla:master Oct 26, 2018
@danburzo danburzo deleted the first branch October 26, 2018 15:59
@erikrose
Copy link
Contributor

It would be great if the URL in the GitHub page also pointed to the docs page rather than the npm package²

Done. :-)

I could include a short Installation section in the readme with the npm package name to supplement the changed URL.

Do you think we need it? The docs already link to the npm page at http://mozilla.github.io/fathom/#get-fathom.

@danburzo
Copy link
Contributor Author

danburzo commented Oct 26, 2018

Awesome. These are all small things, but somehow the previous version gave me a sort of unease I can't explain :-)

Do you think we need it? The docs already link to the npm page at http://mozilla.github.io/fathom/#get-fathom.

Not necessarily. I guess it's just about conforming to the expectations of the typical JS project on GitHub.

@erikrose
Copy link
Contributor

A second pair of eyes is irreplaceable. Thanks for lending them!

@danburzo
Copy link
Contributor Author

For lack of a better place to ask, are there any good first bugs I could warm up to? (Context: I'm working on percollate, which now uses Readability, but I'd like to experiment with Fathom as well)

@erikrose
Copy link
Contributor

erikrose commented Oct 26, 2018 via email

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.

None yet

2 participants