Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Merge Strategy section to README #1284

Merged
merged 7 commits into from
Sep 6, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 responsability to get a proposed PR to pass the build, not primarily the project's maintainers.
vorburger marked this conversation as resolved.
Show resolved Hide resolved

- [ ] 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 have been followed.
vorburger marked this conversation as resolved.
Show resolved Hide resolved

- [ ] All Integrations tests are passing with the new commits.
- [ ] Add required Swagger annotation and updated API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes
vorburger marked this conversation as resolved.
Show resolved Hide resolved

- [ ] 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 mailling list for guidance, if required.)
vorburger marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -318,6 +318,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 @@ -338,6 +340,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