Avoid Cosmetic Changes
Chatwoot generally doesn't accept cosmetic changes and does not add anything substantial to the stability, performance, functionality, or testability.
Cosmetic copy changes or trivial mistakes
Trivial code changes like variable renaming or cosmetic fixes
Copy/Help text changes
Changes suggested by code linters or automated tools
The reasoning behind this decision is due to hidden costs like the following.
Copy/Help text changes cause significant effort for the community translators as they will have to retranslate the string across all the languages. Learn more about the translation process here
It takes away time and energy the team could spend on critical features and bug fixes.
It pollutes the git history as
git blamehits the refactor commits and makes it harder to understand the original decision-making.
To Avoid confusion, please discuss the changes over a GitHub issue before starting the work. The thought process here is inspired by Ruby on Rails Contribution guide.
Raising a pull request
Points to note when you are raising a PR.
Try to make the review cycle short.
Make sure code follows the style guidelines of this project
Perform a self-review of your code.
Commented on the code, particularly in hard-to-understand areas.
Make necessary changes in the documentation.
Verify that the PR does not generate new warnings. Check rails console as well as browser console.
Add tests to prove that the fix is effective or that the feature works.
New and existing unit tests pass with the changes.
Any dependent changes have been merged and published in downstream modules.
For all external and internal contributions to Chatwoot repositories, the team will triage the pull request and assign a PR owner.
At any point in time, the assignee field in Github shows the PR owner responsible for taking some action. The PR owner will work with the PR author to provide review comments, assist in testing or re-assigning the pull request to another owner when required.
PR raised by a team member
The ownership of the PR is on the person who creates the PR. The PT author's responsibility is to get it reviewed, QA'ed, and merge it to production. When a PR is ready for review, the PR author can assign the PR to people from whom they would like to get a review. Once the review is done, the reviewer can un-assign themselves from the ticket. The last reviewer would un-assign themselves and assign it back to the author.
PR raised by a community member
The on-call person would triage the PR and add an assignee to the PR. Ownership is on reviewers to get it merged.
Please ensure that the commit message is a proper sentence before merging a pull request and has full context about the PR in the description. Here is a guide on writing good commit messages. (https://chris.beams.io/posts/git-commit/)
- A properly formed git commit subject line should always be able to complete the following sentence
If applied, this commit will your subject line here
- Capitalize the subject line
Commit Message Structure
A commit message should be structured as follows.
<type>[optional scope]: <description> [optional body] [optional footer(s)]
Type should follow Conventional Commit Specification. You can read more about the type and conventional commit specification here. https://www.conventionalcommits.org/en/v1.0.0-beta.2/#summary
Update product documentation
Please make sure that product docs are updated before merging the PR. In addition, the PR owner should check the requirement of any documentation changes related to the raised PR.
docs-neededlabel to the PR.
Add the content to the docs,
docs-done labelto the PR after completing the documentation.
Add co-authors if applicable
It is essential to add attribution to people who have contributed to the pull request. Please use
Co-authored-by: at the end of the description.
Commits in branches
We use commit squashing to merge a PR to
develop or release branches.
Avoid force-push to remote branches / Pull Requests
Avoid force-pushing your changes when working on a public branch and you face a merge conflict. Rewriting history through
force-push can have the following side effects.
Changes made by others could be lost
It could mess up previous review comments on Github
Other contributors of the branch will have to delete and re-fetch the branch
Hence one should pull the upstream changes, resolve the merge conflict via a merge commit and then push the changes.
Use rebase to avoid merge commits in local
note: The golden rule of git rebase is to never use it on public branches. ref
A merge commit is created if the local and remote branches have different commits when you pull without the
--rebase flag. Therefore, it is good to rebase your local commits.
To avoid typing
--rebase every time you can configure git to use it as default:
git config --global pull.rebase true