Improve contributing guide

[KevinMind]

trafficstars

Following up on https://github.com/faker-js/faker/issues/1413#issuecomment-1298755644 I'd like to improve the contributing guide. Keeping a short list of changes, with a few open questions needing resolution before opening the PR.

• [ ] update contributing guide with fork workflow, using screenshots I snapped.
• [ ] update contributing guide with more details about PRs
• • [ ] should PRs specify tasks that they close/relate to. how so?
• • [ ] anything to put in body?
• • [ ] commits after PR is opened, force or not force?
• • [ ] how to resolve comments before merging?
• • [ ] can you deploy/test code before merging?
• • [ ] before merging, should we squash rebase?
• • [ ] actual merge etiquette. Can someone merge after approvals? Is there a process or steps to follow?
• [ ] Link to release process.

[KevinMind]

Question: Is this normal? When re-requesting review it only allows to re-request from one reviewer. If normal, we should document that behaviour, if not, maybe fix it?

https://user-images.githubusercontent.com/19595165/200165962-909bd0bf-1033-48c5-8e60-5ed727d5e0cc.mov

[ejcheng]

Question: Is this normal? When re-requesting review it only allows to re-request from one reviewer. If normal, we should document that behaviour, if not, maybe fix it?

Screen.Recording.2022-11-06.at.11.35.30.mov

I don't believe this behavior is intended, try refreshing the page to see if it registers.

[ST-DDT]

• update contributing guide with fork workflow, using screenshots I snapped.

We will decide later what to do here.

• should PRs specify tasks that they close/relate to. how so?
• anything to put in body?

We should update the PR template with

Fixes #1513

- #1513

...

• commits after PR is opened, force or not force?

Push commits until the PR is ready, nothing to document here.

• how to resolve comments before merging?

If the issue is explicitly resolved (e.g. typo is fixed) mark the comment as resolved. If not, don't. We will re-review the code for new/resolved issues in the changed code anyway.

• can you deploy/test code before merging?

Document how to use "overwrites".

• before merging, should we squash rebase?

No, do not squash as this complicates the review process. We will squash the PR for you. This doesn't have to be documented as most open source projects do this like this.

• actual merge etiquette. Can someone merge after approvals? Is there a process or steps to follow?

Only the team can merge, so nothing external contributors can do. We have rules when to merge PRs (wait times for more reviews), but these are irrelevant for others and thus don't need to be documented in the contribution guidelines.

• Link to release process.

External contributors cannot release (under our name) and thus this doesn't have to be documented in the contribution guidelines. We will document the release steps, when we update/automate the process in a separate document.

[matthewmayer]

Only the team can merge, so nothing external contributors can do. We have rules when to merge PRs (wait times for more reviews), but these are irrelevant for others and thus don't need to be documented in the contribution guidelines.

Even if there's nothing a contributor can do, it's nice to give people some kind of clarification like "don't worry if you open a PR and after some initial comments, nothing seems to happen for a few weeks, it will take time for the maintainers to review the code, decide to accept it, and then merge."

[ST-DDT]

Team Decision

• update contributing guide with fork workflow, using screenshots I snapped.
• update contributing guide with more details about PRs
  • commits after PR is opened, force or not force?
  • how to resolve comments before merging?
  • can you deploy/test code before merging?
  • before merging, should we squash rebase?
  • actual merge etiquette. Can someone merge after approvals? Is there a process or steps to follow?

We will review existing contribution.md regarding these topics. Additionally, we will create a contribution guide similar to eslint's on our website which will include multiple sections on this topic. This will include the results of the review of the contribution.md Our contribution.md will include a link to our website

• should PRs specify tasks that they close/relate to. how so?
• anything to put in body?

• Extend PR template with hints like

Fixes #<issue>

- #<issue>

Does xyz

This will also be included in the future "how to contribute/PR" section.

• Link to release process.

We will add a section similar to eslint's.