Update contribution documentation for core team members (#3318)

* Update issue tracking docs, re. contributor workflow discussions

* Update docs regarding the PR review & merge process

* Update PR docs with suggestions re. contributor workflow

docs/contributing/committing.rst

@@ -8,6 +8,9 @@ or for anyone interested in the process of getting code committed to Wagtail.
 Code should only be committed after it has been reviewed
 by at least one other reviewer or committer,
 unless the change is a small documentation change or fixing a typo.
+If additional code changes are made after the review, it is OK to commit them
+without further review if they are uncontroversial and small enough that
+there is minimal chance of introducing new bugs.
 
 Most code contributions will be in the form of pull requests from Github.
 Pull requests should not be merged from Github, apart from small documentation fixes,
@@ -42,6 +45,12 @@ You can fix up any small mistakes in the commits,
 such as typos and formatting, as part of the rebase.
 ``git rebase --interactive`` is an excellent tool for this job.
 
+Ideally, use this as an opportunity to squash the changes to a few commits, so
+each commit is making a single meaningful change (and not breaking anything).
+If this is not possible because of the nature of the changes, it's acceptable
+to either squash into a commit or leave all commits unsquashed,
+depending on which will be more readable in the commit history.
+
 .. code-block:: console
 
     $ # Get the latest commits from Wagtail
@@ -119,3 +128,11 @@ The changes are ready to be pushed to ``master`` now.
     $ # Push the commits!
     $ git push upstream master
     $ git branch -d pr/xxxx
+
+When you have made a mistake
+============================
+
+It's ok! Everyone makes mistakes. If you realise that recent merged changes
+have a negative impact, create a new pull request with a revert of the changes
+and merge it without waiting for a review. The PR will serve as additional
+documentation for the changes, and will run through the CI tests.

docs/contributing/issue_tracking.rst

@@ -17,24 +17,31 @@ As soon as a ticket is opened - ideally within one day - a member of the core te
 The possible milestones that it might be assigned to are as follows:
 
 * **invalid** (closed): this issue doesn't identify a specific action to be taken, or the action is not one that we want to take. For example - a bug report for something that's working as designed, or a feature request for something that's actively harmful.
-* **some-day**: the issue is accepted as valid (i.e. it's a bug report for a legitimate bug, or a useful feature request) but not deemed a priority to work on (in the opinion of the core team). For example - a bug that's only cosmetic, or a feature that would be kind of neat but not really essential. Feel free to take it on!
-* **real-soon-now**: the issue is considered important by the core team (roughly speaking: it's a bug or missing feature that's causing headaches for people) but isn't necessarily going to be completed on any set timescale. Feel free to take it on!
-* A specific version number: the issue is important enough that it needs to be fixed in this version.
+* **some-day**: the issue is accepted as valid (i.e. it's a bug report for a legitimate bug, or a useful feature request) but not deemed a priority to work on (in the opinion of the core team). For example - a bug that's only cosmetic, or a feature that would be kind of neat but not really essential. There are no resources allocated to it - feel free to take it on!
+* **real-soon-now**: the issue is considered important by the core team (roughly speaking: it's a bug or missing feature that's causing headaches for people) but isn't necessarily going to be completed on any set timescale. There are no resources allocated to it - feel free to take it on!
+* A specific version number (eg. **1.6**): the issue is important enough that it needs to be fixed in this version. There are resources allocated and/or plans to work on the issue in the given version.
 
 On some occasions it may take longer for the core team to classify an issue into a milestone. For example:
 
 * It may require a non-trivial amount of work to confirm the presence of a bug. In this case, feedback and further details from other contributors, whether or not they can replicate the bug, would be particularly welcomed.
 * It may require further discussion to decide whether the proposal is a good idea or not - if so, it will be tagged "design decision needed".
 
-We will endeavour to make sure that issues don't remain in this state for prolonged periods. Issues and PRs tagged "design decision needed" will be revisited regularly and discussed with at least two core contributors - we aim to review each ticket at least once per release cycle (= 6 weeks).
+We will endeavour to make sure that issues don't remain in this state for prolonged periods. Issues and PRs tagged "design decision needed" will be revisited regularly and discussed with at least two core contributors - we aim to review each ticket at least once per release cycle (= 6 weeks) as part of weekly core team meetings.
 
 Pull requests
 -------------
 
-As with issues, the core team will classify pull requests as soon as they are opened, usually within one day. Unless the change is invalid or particularly contentious (in which case it will be closed or marked as "design decision needed"), it will generally be classified under the next applicable version - the next minor release for new features, or the next patch release for bugfixes - and marked as 'Needs review'. All contributors, core and non-core, are invited to offer feedback on the pull request.
+As with issues, the core team will classify pull requests as soon as they are opened, usually within one day. Unless the change is invalid or particularly contentious (in which case it will be closed or marked as "design decision needed"), it will generally be classified under the next applicable version - the next minor release for new features, or the next patch release for bugfixes - and marked as 'Needs review'.
+
+* All contributors, core and non-core, are invited to offer feedback on the pull request.
+* Core team members are invited to assign themselves to the pull request for review.
 
 Subsequently (ideally within a week or two, but possibly longer for larger submissions) a core team member will merge it if it is ready to be merged, or tag it as requiring further work ('needs work' / 'needs tests' / 'needs docs'). In the latter case, they may also reassign it to a later milestone ('real-soon-now' or 'some-day'). Pull requests that require further work are handled and prioritised in the same way as issues - anyone is welcome to pick one up from the backlog, whether or not they were the original committer.
 
+Rebasing / squashing of pull requests is welcome, but not essential. When doing so, do not squash commits that need reviewing into previous ones and make sure to preserve the sequence of changes. To fix mistakes in earlier commits, use ``git commit --fixup`` so that the final merge can be done with ``git rebase -i --autosquash``.
+
+Core team members working on Wagtail are expected to go through the same process with their own fork of the project.
+
 Release schedule
 ----------------