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

docs: Added table of contents to contributor docs #857

Merged
merged 9 commits into from Mar 30, 2017

Conversation

Projects
None yet
4 participants
@gilbertginsberg
Copy link
Contributor

commented Mar 10, 2017

Fixes #856

@coveralls

This comment has been minimized.

Copy link

commented Mar 10, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b020093 on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@kumar303

This comment has been minimized.

Copy link
Member

commented Mar 10, 2017

Awesome! A TOC will be really helpful. I'm worried about people forgetting to update it though. Can we use a script instead, maybe this one? https://github.com/thlorenz/doctoc

@gilbertginsberg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2017

Oh nice. Sure, I'll look into implementing @kumar303.

@gilbertginsberg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2017

@kumar303: TOC is now generated with doctoc. Nifty tool.

@coveralls

This comment has been minimized.

Copy link

commented Mar 10, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 004c185 on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

gilbertginsberg added some commits Mar 10, 2017

@gilbertginsberg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2017

One thing to note: whoever updates CONTRIBUTING.md will still need to remember to update the TOC by running the command doctoc CONTRIBUTING.md. Unless there's a way to automatically run the command in the background whenever CONTRIBUTING.md is edited? Not sure if that's possible or even makes sense...

@coveralls

This comment has been minimized.

Copy link

commented Mar 10, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling d33eb1f on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@coveralls

This comment has been minimized.

Copy link

commented Mar 10, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling d33eb1f on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@kumar303
Copy link
Member

left a comment

I think this is the right approach. A few comments:


#Picking an issue
development more awesome by contributing to the `web-ext` tool. Here are links to all the sections in this document:

This comment has been minimized.

Copy link
@kumar303

kumar303 Mar 10, 2017

Member

You should put a comment here that says how to run doctoc. Also, you'll need to add it to package.json as a dev requirement and make a script we can use like npm run gen-contributing-toc.

This comment has been minimized.

Copy link
@gilbertginsberg

gilbertginsberg Mar 11, 2017

Author Contributor

Okay, thanks @kumar303. I'll get to work on this.

This comment has been minimized.

Copy link
@gilbertginsberg

gilbertginsberg Mar 11, 2017

Author Contributor

@kumar303: I've made these three changes. Will be alert if others are needed.

@coveralls

This comment has been minimized.

Copy link

commented Mar 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 433f35d on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@coveralls

This comment has been minimized.

Copy link

commented Mar 11, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b9384a6 on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@kumar303
Copy link
Member

left a comment

Thanks, automatically installing the deps will make it easy for other developers to make changes. Can you also add a note to contributing.md about how to change contributing.md? It's a bit meta but developers will need to know how to make changes to it :) The comment you added is excellent but I think the contributing guide should also contain a sentence or two about it.

@gilbertginsberg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2017

Good call re: the note, @kumar303 . I wondered about this.

I debated what the best spot for it would be, and looked around for a repo with something similar. And ultimately went with a new Documentation heading because it seems natural there and perhaps can be of use down the road. But were you suggesting to add it right by to the TOC itself? That was another thought...

@coveralls

This comment has been minimized.

Copy link

commented Mar 12, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling cacda3a on gilbertginsberg:docs/856 into 0c84f70 on mozilla:master.

@kumar303
Copy link
Member

left a comment

Awesome, looks great. Thanks.

@kumar303

This comment has been minimized.

Copy link
Member

commented Mar 12, 2017

@rpl can you think of a fancy way to automatically run the TOC generator script? I can't think of anything that wouldn't have some annoyance to it. It's probably ok to run it manually since editing contributing.md is a rare occurrence.

@kumar303 kumar303 requested a review from rpl Mar 14, 2017

@kumar303

This comment has been minimized.

Copy link
Member

commented Mar 29, 2017

@gilbertginsberg whoops, I almost lost track of this. Can you resolve the conflicts? The toc might need to be updated. After that, I think we can merge it in and just enforce toc generation by word of mouth for now.

@kumar303 kumar303 self-requested a review Mar 29, 2017

@coveralls

This comment has been minimized.

Copy link

commented Mar 30, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling c9203d8 on gilbertginsberg:docs/856 into 03411b3 on mozilla:master.

@coveralls

This comment has been minimized.

Copy link

commented Mar 30, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling c9203d8 on gilbertginsberg:docs/856 into 03411b3 on mozilla:master.

@gilbertginsberg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 30, 2017

@kumar303: Sure thing, just resolved the conflicts.

@kumar303
Copy link
Member

left a comment

Thanks! I'd like to merge this in. We can see how easy it is to keep the toc up to date :)

@kumar303 kumar303 merged commit f0d21f2 into mozilla:master Mar 30, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details

@gilbertginsberg gilbertginsberg deleted the gilbertginsberg:docs/856 branch Mar 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.