chore(maintenance): add project guides #36

profnandaa · 2018-09-25T11:53:55Z

issue template
PR template
contributing guide
maintainers guide

closes #35

JaKXz

This is awesome! I have some suggestions though.

JaKXz · 2018-09-26T05:10:37Z

MAINTAINERS_GUIDE.md

+- At least two Collaborators must approve a pull request before the pull request lands.
+- PRs with commits that don't follow the [conventional commits standard](https://www.conventionalcommits.org) should be re-written when merging.
+- Use Github's _squash and merge_ when landing PRs.
+- CI must pass before landing PRs, needless to day.


typo, needless to say

Good, catch, fixing.

JaKXz · 2018-09-26T05:11:18Z

CONTRIBUTING.md

+  ```
+- Open the pull request, see details in the template.
+- Make any necessary changes after review.
+- In order to land, a Pull Request needs to be reviewed and approved by at least one `c8` Collaborator.


This is different from the maintainers' guide

I think I can remove this; looks like it's creates confusion...

JaKXz · 2018-09-26T05:12:27Z

CONTRIBUTING.md

+
+### Setting up your local environment
+
+- Make sure you have installed the latest version of Node.js


just a minor thing, you could just use 1. for every bullet point and markdown will figure out the numbering, even if we have to add or remove steps later.

JaKXz · 2018-09-26T05:13:21Z

CONTRIBUTING.md

+  ```
+  $ npm run test:snap
+  ```
+- If all is passing, commit your changes following the [conventional commits guideline](https://www.conventionalcommits.org/)


I'd argue this isn't strictly necessary since the maintainers can do this with the squash and merge function. IME this is a barrier to entry for beginners, so I'd put it in the Maintainers' guide.

Makes sense, will move it.

JaKXz · 2018-09-26T05:14:49Z

.github/PULL_REQUEST_TEMPLATE.md

+- [ ] `npm run test:snap` (to update the snapshot)
+- [ ] tests and/or benchmarks are included
+- [ ] documentation is changed or added
+- [ ] commit message follows [conventional commit guidelines](https://www.conventionalcommits.org/)


as mentioned below, this can be optional for the contributor and is really up to the merger to enforce with GH's squash and merge function.

profnandaa · 2018-09-26T06:02:55Z

@JaKXz -- thanks for the review Jason, fixed 👍

coveralls · 2018-09-26T06:04:37Z

Pull Request Test Coverage Report for Build 51

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 90.934%

Totals
Change from base Build 48:	0.0%
Covered Lines:	214
Relevant Lines:	218

💛 - Coveralls

JaKXz · 2018-09-30T00:25:25Z

CONTRIBUTING.md

+## Issues
+
+- You can open [issues here](https://github.com/bcoe/c8/issues), please follow the template guide.
+- You can come over to the `node-tooling/c8` channel for discussions, [follow this link](https://devtoolscommunity.herokuapp.com/) to request for an invite.


minor nit: drop for

JaKXz · 2018-09-30T00:26:38Z

MAINTAINERS_GUIDE.md

+- At least two Collaborators must approve a pull request before the pull request lands.
+- PRs with commits that don't follow the [conventional commits standard](https://www.conventionalcommits.org) should be re-written when merging (squash and merge).
+- Use Github's _squash and merge_ when landing PRs.
+- CI must pass before landing PRs, needless to say.


Honestly, if it's needless to say, it doesn't need to be here. I should've said that the first time, sorry.

MAINTAINERS_GUIDE.md

JaKXz · 2018-09-30T00:29:43Z

MAINTAINERS_GUIDE.md

+
+- Label the issues appropriately, see the list of labels and their description [here](https://github.com/bcoe/c8/labels)
+- Be welcoming to first-time contributors, identified by the GitHub `first-time contributor` badge.
+- At least two Collaborators must approve a pull request before the pull request lands.


Just curious if @bcoe thinks this project is at that level of maturity already... I wouldn't want to block a contribution waiting for that 2nd review.

Maybe this could be relaxed to should be approved by 1-2 collaborators before merging? 🤷‍♂️

I think 1 is decent; and looks like that's how it has been... rewriting.

JaKXz · 2018-09-30T00:30:30Z

Thanks @profnandaa - I have a few more thoughts... sorry for the piece-meal reviews!

profnandaa · 2018-09-30T05:58:00Z

No worries sir :) Will check it out.

-na

On Sun, Sep 30, 2018 at 3:30 AM Jason Kurian ***@***.***> wrote: Thanks @profnandaa <https://github.com/ProfNandaa> - I have a few more thoughts... sorry for the piece-meal reviews! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAP8kUxqC2uqxt5-Hrw3eLrLtsvrHPrXks5ugBCmgaJpZM4W4c0u> .

-- Sent from a tiny device while on the move.

- issue template - PR template - contributing guide - maintainers guide closes bcoe#35

profnandaa · 2018-10-01T03:23:01Z

@JaKXz -- thanks, done.

bcoe · 2018-10-01T18:36:34Z

.github/PULL_REQUEST_TEMPLATE.md

+##### Checklist
+<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->
+- [ ] `npm test`, tests passing
+- [ ] `npm run test:snap` (to update the snapshot)


perhaps add here, if any new tests have been added.

bcoe

this is very good, I'm comfortable landing; @JaKXz any more blockers?

JaKXz

I'm done now, thanks @profnandaa :)

- issue template - PR template - contributing guide - maintainers guide closes bcoe#35

profnandaa requested a review from bcoe September 25, 2018 11:54

profnandaa force-pushed the chore-35-maintenance branch from 0a92e9d to 7285e46 Compare September 25, 2018 11:57

JaKXz suggested changes Sep 26, 2018

View reviewed changes

profnandaa force-pushed the chore-35-maintenance branch from 7285e46 to c2f9be3 Compare September 26, 2018 06:03

profnandaa mentioned this pull request Sep 26, 2018

Integration tests sometimes failing #27

Closed

JaKXz reviewed Sep 30, 2018

View reviewed changes

MAINTAINERS_GUIDE.md Show resolved Hide resolved

JaKXz reviewed Sep 30, 2018

View reviewed changes

chore(maintenance): add project guides

fcdc964

- issue template - PR template - contributing guide - maintainers guide closes bcoe#35

profnandaa force-pushed the chore-35-maintenance branch from c2f9be3 to fcdc964 Compare October 1, 2018 03:22

bcoe reviewed Oct 1, 2018

View reviewed changes

bcoe approved these changes Oct 1, 2018

View reviewed changes

JaKXz approved these changes Oct 1, 2018

View reviewed changes

profnandaa merged commit b0d1b7b into bcoe:master Oct 2, 2018

profnandaa deleted the chore-35-maintenance branch October 2, 2018 04:43

mcknasty pushed a commit to mcknasty/c8 that referenced this pull request Feb 2, 2024

chore(maintenance): add project guides (bcoe#36)

9401072

- issue template - PR template - contributing guide - maintainers guide closes bcoe#35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(maintenance): add project guides #36

chore(maintenance): add project guides #36

profnandaa commented Sep 25, 2018

JaKXz left a comment

JaKXz Sep 26, 2018

profnandaa Sep 26, 2018

JaKXz Sep 26, 2018

profnandaa Sep 26, 2018

JaKXz Sep 26, 2018

JaKXz Sep 26, 2018

profnandaa Sep 26, 2018

JaKXz Sep 26, 2018

profnandaa commented Sep 26, 2018

coveralls commented Sep 26, 2018

JaKXz Sep 30, 2018 •

edited

Loading

profnandaa Oct 1, 2018

JaKXz Sep 30, 2018

JaKXz Sep 30, 2018

profnandaa Oct 1, 2018

JaKXz commented Sep 30, 2018

profnandaa commented Sep 30, 2018 via email

profnandaa commented Oct 1, 2018

bcoe Oct 1, 2018

bcoe left a comment

JaKXz left a comment


		### Setting up your local environment

		- Make sure you have installed the latest version of Node.js

chore(maintenance): add project guides #36

chore(maintenance): add project guides #36

Conversation

profnandaa commented Sep 25, 2018

JaKXz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

profnandaa commented Sep 26, 2018

coveralls commented Sep 26, 2018

Pull Request Test Coverage Report for Build 51

💛 - Coveralls

JaKXz Sep 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaKXz commented Sep 30, 2018

profnandaa commented Sep 30, 2018 via email

profnandaa commented Oct 1, 2018

Choose a reason for hiding this comment

bcoe left a comment

Choose a reason for hiding this comment

JaKXz left a comment

Choose a reason for hiding this comment

JaKXz Sep 30, 2018 •

edited

Loading