All very good points Chris. >> > 3. Merge Conventions / Protected Branch:
> Nice idea. The one exception is that committers sometimes have to tag > and push individual commits when making releases. We will definitely have to check how this works with current workflows defined in coho and in other places. > I would favor a place where a contributor can mark if s/he would > recommend the committer should *not* use the squash option. That of course could be defined in the contributor guidelines or PR template (although there it might be a bit confusing to new users that don't know what this is talking about). Do you know how other project handle that? In the end it is always the decision of the committer how contributed code makes it into the code base, so having good guidelines for us would probably be the best solution, right? -J 2018-08-07 21:36 GMT+02:00 Chris Brody <chris.br...@gmail.com>: >> 1. Issue template: >> [...] >> I think we should definitely use this for Bug Report, Feature Request, >> Support Question. What sections should we suggest in our templates? > > All suggestions look good, should probably give proper attribution. > >> 2. Pull Request template: >> >> I think we can drop the JIRA/issue requirement from the checklist >> completely - as a PR on Github is always the starting point of code >> changes now. If there is an (Github) issue the PR resolves, it should >> of course be linked (`closes #123` or similar), but this is not a >> requirement. >> >> Similar about commit message conventions - I don't think we need an >> issue ID in there as all code changes will be done via PRs which via >> "squash + merge" will include PR ID in the commit message (again >> fastlane as an example: >> https://github.com/fastlane/fastlane/commits/master) >> >> I would suggest we use something similar to this, which explicitly >> asks for running the tests and writing documentation: >> https://github.com/fastlane/fastlane/blob/master/.github/PULL_REQUEST_TEMPLATE.md >> >> What do you think regarding PR template? > > Looks nice in general, again we should probably give proper > attribution upon reuse. > > It might be good to require a checkbox for developers certificate of > origin (developercertificate.org) like at: > https://github.com/papercss/papercss/blob/master/.github/PULL_REQUEST_TEMPLATE.md > >> 3. Merge Conventions / Protected Branch: >> >> Connected to all that is my suggestion to protect the `master` branch >> so that by default nobody can commit there - all changes have to be >> made via Pull Requests. > > Nice idea. The one exception is that committers sometimes have to tag > and push individual commits when making releases. > > A possible workaround would be to say that we always start a release > branch before publishing an actual release, like they seem to do in > Node.js. Could keep things a little cleaner. > >> Pull Requests are by default merged via the >> "Squash + Merge" button / functionality so that all commits are >> squashed into one clean commit per change. This also enforces the >> commit message structure I posted above. (Of course committers can >> choose to _not_ use Squash + Merge if appropriate for the PR - e.g. >> when cherry picking commits from a release branch or similar). > > I would favor a place where a contributor can mark if s/he would > recommend the committer should *not* use the squash option. > > I am starting to favor a choice between squash merge and merge commit. > I think these choices keep things a little more clear in the history > and would reliably mark the PR, unlike "rebase merge". > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org > For additional commands, e-mail: dev-h...@cordova.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
