diff --git a/Makefile b/Makefile index fafa77efcf..f29388923a 100644 --- a/Makefile +++ b/Makefile @@ -156,7 +156,7 @@ download-from-job: $(BRANCH_DIR) branch-check -n documentation-$(subst /,-,$(BRANCH)) -D $(BRANCH_DIR)/source ifeq ($(BRANCH),) - build: $(BUILD_DIR)/docs dynamic-setup + build: static-setup dynamic-setup @echo Building to $(SITE_DIR) @[ ! -f $(BUILD_DIR)/mkdocs.yml ] && \ { echo "Can't build. '$(BUILD_DIR)/mkdocs.yml' not found" ; exit 1 ; } || : diff --git a/docs/Development/Policies-and-Procedures/Code-Contribution.md b/docs/Development/Policies-and-Procedures/Code-Contribution.md index 415f720fae..b1c249f99e 100644 --- a/docs/Development/Policies-and-Procedures/Code-Contribution.md +++ b/docs/Development/Policies-and-Procedures/Code-Contribution.md @@ -1,16 +1,10 @@ ---- -title: Code Contribution -pageid: 52069715 ---- - -# Overview - +# Code Contribution All code management/contribution/review processes will be handled with [GitHub Asterisk Pull Requests](https://github.com/asterisk/asterisk/pulls) and [GitHub Testsuite Pull Requests](https://github.com/asterisk/testsuite/pulls). Note that Asterisk and Testsuite pull requests must be created in their own repositories. -# Code Contribution Process +## Code Contribution Process -## Install the [GitHub CLI "gh"](https://cli.github.com) tool +### Install the [GitHub CLI "gh"](https://cli.github.com) tool While not strictly required, using the "[gh](https://cli.github.com)" tool to manage the process will make things much easier. The package is available in most distribution's package management systems as "gh". @@ -32,7 +26,7 @@ While not strictly required, using the "[gh](https://cli.github.com)" tool to ma 3. Select "paste an authentication token" as the authentication method. 4. Run `gh auth setup-git` to allow git itself to use the gh authentication method. -## Fork the Repositories +### Fork the Repositories Contributors must fork the [asterisk/asterisk](https://github.com/asterisk/asterisk) and [asterisk/testsuite](https://github.com/asterisk/testsuite) repositories into their own GitHub account. If you clone the asterisk/asterisk or asterisk/testsuite repositories directly to your development system you will NOT be able to submit pull requests from it. If you have existing repositories cloned from Gerrit, please don't try to re-use the clone by changing the remotes. Similarly, if you happen to already have a fork of the repos in your GitHub account, it's a good idea to delete those forks and re-create them. Starting fresh will ensure you don't have issues later. @@ -48,7 +42,7 @@ Git Remotes will automatically be created for both your fork and the upstream re In each of the clones, run `gh repo set-default`. Select either asterisk/asterisk or asterisk/testsuite as appropriate. They should be the defaults but check anyway. Also run `git config user.email` and `git config user.name` in each of the repos to make sure they're correct. At a minimum, user.email should match one of the emails you've added to your GitHub account. -## Do Work +### Do Work Checkout the **HIGHEST** VERSION branch to which your work will apply ('master', '20', '18', etc.), update it to match the upstream repo, then push it to your fork. @@ -76,38 +70,39 @@ Now make your change and test locally. [//]: # (end-note) -## Commit +### Commit -Commit messages should follow the guidelines established in [Commit Messages](/Development/Policies-and-Procedures/Commit-Messages). That page will be updated as follows after the cut-over. - -* You can use [GitHub Flavored Markdown](https://github.github.com/gfm/) in your commit messages to make them look nicer. -* If there isn't an open GitHub issue for your work, open one now. If there was an existing JIRA issue, you must still open a new GitHub issue. -* To reference an issue, use a `Resolves: #` header on a separate line. -* To make users aware of possible breaking changes on update, use an `UpgradeNote: ` header starting on a new line and ending with an empty line. -* To make users aware of a new feature or significant fix, use a `UserNote: ` header starting on a new line and ending with an empty line. - -The new headers create entries near the top of the ChangeLogs. +Commit messages should follow the guidelines established in [Commit Messages](/Development/Policies-and-Procedures/Commit-Messages). --- Sample Commit Message -```text -app_something: Add some new capability +``` +app_foo.c: Add new 'x' argument to the Foo application -app_something has been updated to include new feature "X". To configure, -edit app_something.conf and add an "X = something" to the "general" -section. +The Foo application now has an addition argument 'x' that can manipulate +the output RTP stream of the remote channel by causing it to pause for +a configured amount of time, at a configured interval and a configured +number of times. There's no real use for this other as an example of +how to format a commit message. -Resolves: #456 +The code required changes to a number of other modules and is fairly +invasive and poorly written. It also required removing an option from +the existing OldFoo application. -UpgradeNote: The old "X" option in app_something.conf has been renamed to -"Z" to better reflect its true purpose. +Fixes: #666 -UserNote: app_something has been updated to include new feature "X". +UserNote: The Foo dialplan application now takes an additional argument +'x(a,b,c)' which will cause the remote channel to pause RTP output for +'a' milliseconds, every 'b' milliseconds, a total of 'c' times. + +UpgradeNote: The X argument to the OldFoo application has been removed +and will cause an error if supplied. ``` -## Test and check for Cherry-pick-ability + +### Test and check for Cherry-pick-ability This should go without saying but test your change locally to make sure it does what you think it should and that it doesn't break anything else. If it passes and it needs to be cherry-picked to other branches, test cherry-picking now. Create a new branch off the cherry-pick target branch, cherry-pick your change into it then compile and test. If it picks cleanly and passes your tests, you can just delete the branch as you won't be creating additional pull requests for it. If it doesn't apply or pass the tests, you have two options... @@ -116,7 +111,7 @@ This should go without saying but test your change locally to make sure it does You should always use option 1 when possible. Unlike Gerrit, GitHub was never designed to handle pushing the same change to multiple branches. There's no easy way to relate the pull requests and even the GitHub UI doesn't indicate what the target branch is. This makes it labor intensive for us to manage. -## Create a Pull Request +### Create a Pull Request When you've finished your work and committed, you can create a new pull request by running `gh pr create --fill --base 18`. The `--fill` option sets the pull request description to the same as the commit message and the `--base` option indicates which asterisk branch the pull request is targeted for. This is similar to running `git review 18` to create a new Gerrit review. When prompted where the new branch should be pushed, choose your fork, NOT the upstream repo. @@ -125,16 +120,11 @@ You _may_ create a pull request that has more than 1 commit if the commits rep !!! warning You must **never** add commits to a pull request that fix issues in earlier commits in that PR. -If you want your change to be automatically cherry-picked to other branches, you'll need to add a comment to your pull request. Head over to and open your PR. Add a comment with a `cherry-pick-to: "` header line for each branch. For example, if the PR is against the master branch and you want it cherry-picked down to 20 and 18... - ---- - -PR Cherry-Pick Request comment example +If you want your change to be automatically cherry-picked to other branches, you'll need to add a comment to your pull request. Head over to and open your PR. Add a comment with a `cherry-pick-to: "` header line for each branch. For example, if the PR is against the master branch and you want it cherry-picked down to 20 and 18, add a comment with the following: ```text cherry-pick-to: 20 cherry-pick-to: 18 - ``` Each branch must be on a separate line and don't put anything else in the comment. When all the PR tests and checks have passed, an Asterisk Core developer will trigger the cherry-pick test process which will look for that comment. If the commit can't be cherry-picked cleanly to the branches you indicated or the tests fail, none of the commits will be merged. This is why it's important for you to make sure your commit cherry-picks cleanly before submitting the first pull request. @@ -146,16 +136,19 @@ If you don't need your PR automatically cherry-picked, please add a comment stat [//]: # (end-note) +!!! warning + Don't add the cherry-pick-to lines to the commit message or the PR description. They're only searched for in PR comments. + !!! warning **If you change your mind and don't want your PR automatically cherry-picked, edit the comment and replace the "cherry-pick-to" lines with a single `cherry-pick-to: none` line** Don't use formatting or other means to say "nevermind". The automation might not understand. [//]: # (end-warning) -# Pull Request Review Process +## Pull Request Review Process As with Gerrit reviews, a new PR triggers a set of tests and checks. If you browse to your PR and scroll to the bottom, you'll see the status of those checks listed. There are some differences to Gerrit however. -## New Contributor License Agreement +### New Contributor License Agreement Every contributor will be required to sign a new Contributor License Agreement before their first PR can be merged. One of the PR checks will be "license/cla" which looks like this... @@ -163,19 +156,19 @@ Every contributor will be required to sign a new Contributor License Agreement b which indicates that you haven't signed it yet. Click the "Details" link to be taken to the page that allows you to fill out the form and sign. Acceptance is automatic so there should be no delay and you only have to do this once. YOUR PR CANNOT BE MERGED UNTIL THIS CHECK IS COMPLETED. -## Automated Tests +### Automated Tests GitHub gives us access to more resources for testing than we've ever had so instead of running the Unit tests at PR submission and the Gate/Testsuite tests when the change has been approved, we run both the Unit and Gate/Testsuite tests immediately upon submission. The Unit tests run as a single job/check but the Gates are broken up into multiple jobs/checks so they can be run in parallel. When each check is completed, a comment will be added to the PR with the result and each check will have it's own line in the checks summary a the bottom of the PR. All checks must pass or be deemed "false alarms" before a PR can be merged. -## Reviewing a change +### Reviewing a change GitHub has two types of Pull Request comments. -### General Comments +#### General Comments These are comments you leave on the main PR page. They are just that...comments. They have no bearing on whether the PR can be merged. If you have general comments or questions about a PR, this is where you leave them. The Gerrit equivalent is clicking the "REPLY" button at the top of the review and leaving a comment without changing your vote. -### Review Comments +#### Review Comments These are comments you have about the code itself. These are left by clicking on the 'Files changed" tab at the top of the PR, then... @@ -189,7 +182,7 @@ These are comments you have about the code itself. These are left by clicking o [//]: # (end-note) -## Address Review Comments and Test Failures +### Address Review Comments and Test Failures If you need to make code changes to address comments or failures, the process is much like it is with Gerrit... @@ -200,16 +193,16 @@ If you need to make code changes to address comments or failures, the process is This will force push the commit to your fork first, then update the PR with the new commit and restart the testing process. !!! warning -Unlike Gerrit, GitHub allows you to have multiple commits for a pull request but it was intended to allow a PR to be broken up into multiple logical chunks, not to address review comments. Using multiple commits to address review comments will make the commit history messy and confusing. Please amend and force push for them otherwise the PR will be rejected. +Unlike Gerrit, GitHub allows you to have multiple commits for a pull request but it was intended to allow a PR to be broken up into multiple logical chunks, not to address review comments. Using multiple commits to address review comments will make the commit history messy and confusing. Please amend and force push otherwise the PR will be rejected. [//]: # (end-warning) -## Cherry-Pick Tests +### Cherry-Pick Tests When an Asterisk Core Team member believes the PR is ready, they'll add a `cherry-pick-test` label to the PR that will jobs to run that check that the cherry-pick applies cleanly to the other branches and run the same automated tests that ran for the original PR. These tests must pass (or be deemed false alarms) for the PR to be eligible for merging. -## Merge +### Merge When an Asterisk Core Team member believes the PR is ready for merging, they'll approve the merge which will cause the original PR to merge into its target branch and cause the change to be cherry-picked into each of the cherry-pick target branches. diff --git a/docs/Development/Policies-and-Procedures/Commit-Messages.md b/docs/Development/Policies-and-Procedures/Commit-Messages.md index ac356eb03a..e08a30072d 100644 --- a/docs/Development/Policies-and-Procedures/Commit-Messages.md +++ b/docs/Development/Policies-and-Procedures/Commit-Messages.md @@ -1,29 +1,17 @@ ---- -title: Commit Messages -pageid: 3702833 ---- +# Commit Messages A commit message serves to notify others of the changes made to the Asterisk source code, both in a historical sense and in the present. Commit messages are **incredibly** important to the continued success of the Asterisk project. Developers maintaining the Asterisk project in the future will often only have your commit message to guide them in why a particular change was made. For non-developers, archives containing commit messages may be used when searching for fixes to a particular bug. Be sure that the information contained in your message will help them out. - - - !!! warning Follow These Guidelines Commit messages are part of your code change. Committing code with a poorly written commit message creates a maintenance problem for everyone in the Asterisk project. - [//]: # (end-warning) +This page describes the expected format for commit messages used when submitting code to the Asterisk project. See [Code Contribution](/Development/Policies-and-Procedures/Code-Contribution) for more information about pushing your commit for review. -This page describes the expected format for commit messages used when submitting code to the Asterisk project. See [Gerrit Usage](/Development/Policies-and-Procedures/Code-Contribution) for more information about pushing your commit for review. - -On This Page - - -Commit Message Body -=================== +## Commit Message Body ### Basic Format @@ -34,7 +22,7 @@ The following illustrates the basic outline for commit messages: - + ``` @@ -45,7 +33,7 @@ app_foo: Fix crash caused by invalid widget frobbing. ``` -Some commit history viewers treat the first line of commit messages as the summary for the commit. In addition, the Asterisk project uses many scripts that parse commit messages for a variety of purposes. So, an effort should be made to format our commit messages in this fashion. The verbose description may contain multiple paragraphs, itemized lists, etc. *Always end the first sentence (and any subsequent sentences) with punctuation.* +Most commit history viewers treat the first line of commit messages as the summary for the commit. In addition, the Asterisk project uses many scripts that parse commit messages for a variety of purposes. So, an effort should be made to format our commit messages in this fashion. The verbose description may contain multiple paragraphs, itemized lists, etc. *Always end the first sentence (and any subsequent sentences) with punctuation.* Commit messages should be wrapped at 72 columns. @@ -53,30 +41,25 @@ Note that for trivial commits, such as fixes for spelling mistakes, the verbose ### GitHub Flavored Markdown -Since we've moved to a complete GitHub SCM solution, commit messages will automatically be rendered as markdown when viewed on GitHub. Feel free to use markdown intentionally, especially to format code snippets, but also be aware that things you used to put in commit messages might unintentionally be rendered as markdown and be improperly formatted. Consider that dashes and underscores might unintentionally be rendered as strike-through or bold text. - -Special Tags for Commit Messages -================================ +Since we've moved to a complete GitHub SCM solution, commit messages will automatically be rendered as [GitHub Flavored Markdown](https://github.github.com/gfm/) when viewed on GitHub. Feel free to use markdown intentionally, especially to format code snippets, but also be aware that things you used to put in commit messages might unintentionally be rendered as markdown and be improperly formatted. Consider that dashes and underscores might unintentionally be rendered as strike-through or bold text. -GitHub and our release process support several commit message trailers. The trailer name MUST start on a new line and each should be separated by a blank line. If specified at all, the trailers listed below MUST be the last items in the commit message. If you specify any other trailers, including ones that were formerly acceptable, they will become part of the official trailer they follow. so, if you insist on adding trailers like `Signed-Off-By` or `Reported-By` they MUST come before the fist of the official trailers. +### Special Trailers for Commit Messages -### Issue Referencing +GitHub and our release process support several commit message trailers that are used by the change log generation process. The trailer name MUST start on a new line, be followed by a colin (`:`) and each should be separated by a blank line. If specified at all, the trailers listed below MUST be the last items in the commit message. -To have a commit noted in an issue and to also close the issue, reference the issue with a `Resolves` or `Fixes` commit message trailer as follows: +!!! warning + If you specify any other trailers, including ones that were formerly acceptable, they will become part of the official trailer they follow. So, if you insist on adding trailers like `ASTERISK-nnnnn`, `Signed-Off-By` or `Reported-By` they MUST come BEFORE the first of the official trailers. -``` -Resolves: #45 -or -Fixes: #45 - -``` +Current official trailers: -The trailer must start at the beginning of a new line and contain nothing on the line after it. The colon separator is important for the automation and the only spaces allowed are between the colon and the hash sign. The regex for this is `^(Resolves|Fixes):\s*[#][0-9]+`. +* **Resolves**: To reference a GitHub issue, use a `Resolves: #` trailer. This causes GitHub to automatically close the isse when the PR is merged, and it adds the commit to the list of issues closed in the release change log. +* **Fixes**: Synonym for Resolves. You can use either. +* **UpgradeNote**: To make users aware of possible breaking changes on update, use an `UpgradeNote: ` trailer. +* **UserNote**: To make users aware of a new feature or significant fix, use a `UserNote: ` trailer. -Upgrade and User Notes ----------------------- +Any user-affecting change (new feature, change to CLI commands, etc) must be documented with a `UserNote:` trailer. Any breaking change (change to dialplan application or function arguments, API change, etc.) must be documented with an `UpgradeNote:` trailer. Those trailers cause special notes to be output in the change log in addition to the full commit message. -With the migration to GitHub, Changelog and Upgrade notes are no longer supplied separately in the `doc/CHANGES-staging` and `doc/UPGRADE-staging` directories. Instead they should be supplied in the commit message as trailers. Any user-affecting change (new feature, change to CLI commands, etc) must be documented with a `UserNote:` trailer. Any breaking change (change to dialplan application or function arguments, API change, etc.) must be documented with an `UpgradeNote:` trailer. Those trailers cause special notes to be output in the change log in addition to the full commit message. Example complete commit message: +## Example complete commit message: ``` app_foo.c: Add new 'x' argument to the Foo application @@ -102,7 +85,3 @@ and will cause an error if supplied. ``` - - - -