Skip to content

Commit

Permalink
add Merge Strategy section to README (apache#1284)
Browse files Browse the repository at this point in the history
includes improved PR Template (and move commit message blurb in README)
  • Loading branch information
vorburger committed Sep 6, 2020
1 parent 349367a commit 4b435c7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
21 changes: 13 additions & 8 deletions .github/PULL_REQUEST_TEMPLATE.MD
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
## Description
Describe the changes made and why they were made. Ignore if these details are present on the associated Jira ticket

Describe the changes made and why they were made.

Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).


## Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

- [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests

- [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

- [ ] API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm has been updated with details of any API changes.
- [ ] Create/update unit or integration tests for verifying the changes made.

- [ ] Integration tests have been created/updated for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

- [ ] All Integrations tests are passing with the new commits.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes

- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ Logging Guidelines
Pull Requests
-------------

We request that your commit message include a FINERACT JIRA issue, recommended to be put in parenthesis add the end of the first line. Start with an upper case imperative verb (not past form), and a short but concise clear description. (E.g. _Add enforced HideUtilityClassConstructor checkstyle (FINERACT-821)_ or _Fix inability to reschedule when interest accrued larger than EMI (FINERACT-1109)_ etc.).

If your PR is failing to pass our CI build due to a test failure, then:

1. Understand if the failure is due to your PR or an unrelated unstable test.
Expand All @@ -343,6 +345,17 @@ them in different commits. This helps review to review your code faster.

We have an automated Bot which marks pull requests as "stale" after a while, and ultimately automatically closes them.


Merge Strategy
--------------

This project's committers typically prefer to bring your Pull Requests in through _Rebase and Merge_ instead of _Create a Merge Commit_. (If you are unfamiliar with GitHub's UI re. this, note the somewhat hidden little triangle drop-down at the bottom of PR, visible only to committers, not contributors.) This avoids the "merge commits" which we consider to be somewhat "polluting" the projects commits log history view. We understand this doesn't give an easy automatic reference to the original PR (which GitHub automatically adds to the Merge Commit message it generates), but we consider this an only very minor inconvenience; it's typically relatively easy to find the original PR even just from the commit message, and JIRA.

We expect most proposed PRs to typically consist of a single commit. Committers may use _Squash and merge_ to combine your commits at merge time, and if they do so will rewrite your commit message as they see fit.

Neither of these two are hard absolute rules, but mere conventions. Multiple commits in single PR make sense in certain cases (e.g. branch backports).


Dependency Upgrades
-------------------

Expand Down

0 comments on commit 4b435c7

Please sign in to comment.