Code Submission Guidelines
We've enacted standards for commits and pull requests to effectively manage the project over time. We expect all code contributed to follow these standards. If your code doesn't follow them, we will kindly ask you to resubmit it in the correct format.
Git Commits
We follow Angular's code contribution style with precise rules for formatting git commit messages. This leads to more readable messages that are easy to follow when looking through the project history. We will also use commit messages to generate the axe Changelog document.
A detailed explanation of Angular's guidelines and conventions can be found on Google Docs.
Commit Message Format
Each commit message should consist of a header, a body and a footer. The header has a special format that includes a type, a scope and a subject. Here's a sample of the format:
<type>(<scope>): <subject>
<BLANK LINE>
<body>
<BLANK LINE>
<footer>
Here's an example:
perf(rule): improve speed of color contrast rules
Use async process to compare elements without UI lockup
Closes issue #1
Note: We do not link issues to be closed as we have our QA team verify the issue is resolved before closing. Instead use Closes issue # to link to the issue the pr resolves but won't close it once merged.
Commit messages should be 100 characters or less to make them easy to read on Github and various git tools.
How to structure your commits:
Type
Must be one of the following:
- feat: A new feature
- fix: A bug fix
- docs: Documentation only changes
- style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
- refactor: A code change that neither fixes a bug nor adds a feature
- perf: A code change that improves performance
- test: Adding missing tests
- chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
- ci: Changes or fixes to CI configuration such as CircleCI
Scope
The scope specifies the place of the commit change in the codebase along with the type. It could
reference a rule, a commons file, or anything really. E.g. feat(rule) or
test(commons/aria). It would help us call to out rule changes in our changelog with rule used as the scope.
If the scope is too broad to summarize, use the type only and leave off the parentheses. E.g. type: some subject. Keep in mind that a long scope often pushes your commit message over 100 characters. Brevity is helpful for everyone!
Subject
The subject contains succinct description of the change:
- use the imperative, present tense: "change" not "changed" nor "changes"
- don't capitalize first letter
- no dot (.) at the end
Body
Use the imperative, present tense: "change" not "changed" nor "changes", just like the subject. Include the motivation for the change and contrast it with how the code worked before.
Footer
Reference any issue that this commit closes with its fully qualified URL to support both Bitbucket and Github.
If needed, the footer should contain any information about Breaking Changes. Deprecation notices or breaking changes in the Changelog should inform users if they'll need to modify their code after this commit.
A breaking change should be noted with BREAKING CHANGE: (all caps, followed by a colon) and a message.
feat(rules): remove deprecated rules
BREAKING CHANGE: remove rules: th-has-headers, checkboxgroup, radiogroup
Submitting a pull request
We want to keep our commit log clean by avoiding merge messages in branches. Before submitting a pull request, make sure your branch is up to date with the develop branch by either:
- Pulling from develop before creating your branch
- Doing a rebase from origin/develop (will require a force push on your branch)
To rebase from origin/develop if we've pushed changes since you created your branch:
git checkout your-branch
git fetch
git rebase origin/develop
git push origin head -f
Merging a pull request
If a pull request has many commits (especially if they don't follow our commit policy), you'll want to squash them into one clean commit.
In the Github UI, you can use the new Squash and Merge feature to make this easy. If there are merge conflicts preventing this, either ask the committer to rebase from develop following the PR submission steps above, or use the manual method below.
To apply a pull request manually, make sure your local develop branch is up to date. Then, create a new branch for that pull request.
Create a temporary, local branch:
git checkout -b temp-feature-branch
Run the following commands to apply all commits from that pull request on top of your branch's local history:
curl -L https://github.com/dequelabs/axe-core/pull/205.patch | git am -3
If the merge succeeds, use git diff origin/develop to review all the changes that will happen post-merge.
Squashing everything into one commit
Before merging a pull request with many commits into develop, make sure there is only one commit representing the changes in the pull request, so the git log stays lean. We particularly want to avoid merge messages and vague commits that don't follow our commit policy (like Merged develop into featurebranch or fixed some stuff).
You can use git's interactive rebase to manipulate, merge, and rename commits in your local history. If these steps are followed, a force push shouldn't be necessary.
Do not force push to develop or master under any circumstances.
To interactively rebase all of your commits on top of the latest in develop, run:
git rebase --interactive origin/develop
This brings up an interactive dialog in your text editor. Follow the instructions to squash all of your commits into the top one. Rename the top one.
Once this is done, run git log and you will see only one commit after develop, representing everything from the pull request.
Finally, pull from develop with rebase to put all of our local commits on top of the latest remote.
git pull --rebase origin develop
You can then push the latest code to develop (note that force push isn't needed if these steps are followed):
git push origin develop
Writing Integration Tests
For each rule, axe-core needs to have integration tests. These tests are located in tests/integration. This directory contains two other directories. rules, which contains integration tests that can be run on a single page, and full which contains tests that can only be tested by running on multiple pages.
Ensure that for each check used in the rule, there is an integration test for both pass and fail results. Integration tests put in rules can be described using simple code snippets in an HTML file, and a JSON file that describes the expected outcome. For full tests, a complete Jasmine test should be created, including at least one HTML file that has the tested code, and a JS file that has the test statements.