WIP
Table of Contents |
---|
Getting ready to contribute to NetBeans
Bootstrapping (need to be done only once)
Adhere to the ASF Code of Conduct
Please read the Apache Software Foundation Code of Conduct and adhere to it. Pull Requests that violate the ASF Code of Conduct will be rejected.
Your PR may take time to be reviewed by a committer, specially during release phases, so be patient.
Initial Git setup
Please make sure git config contains your name and email. You can quickly check your system wide config by typing:
git config --list --global
If the information is not correct you can update it with:You will not have write permission to github apache mirror, you need to
fork https://github.com/apache/incubator-netbeans to your own repositories.
You need to clone the forked repository and setup name and mail. This also may help git rebase to fullfil its task.
git config --global user.name "John Doe"
git config --global user.email "john@doe.org"
-global can be removed if you want to setup only the current repository.
(it is also possible to access the global git config file via NetBeans, Team → Git → Open Global Configuration)
(it is also possible to setup this on a per-repository basis, for that, setup the repository first following the steps below, then use git config
without the --global
option)
Initial Repository setup:
On a high level, you want to work using local branches and mirror them to your NetBeans fork on github. Once you are happy with a change, you would open a pull request (PR) to offer the change to be integrated into the main repository.
1) fork the NetBeans repository on GitHub (https://github.com/apache/netbeans) (if you are asked to enable github actions, leave them disabled for now)
2) clone your newly forked repository into a local copy
Assuming your fork is at https://github.com/YOUR_USERNAME/netbeans.git you can clone the forked repository to a local folder using the following command
git clone https://github.com/YOUR_USERNAME/netbeans.git
Optional: your remote fork carries the origin alias. To remember the main repository too, you can add it as upstream alias:Also add the Apache NetBeans incubator project as your upstream in order to submit PRs
git remote add upstream https://github.com/apache/incubator-netbeans.git
Note: the guide here will always use full URIs instead of aliases to avoid accidents due to ambiguities or differences in local setups.
Use branches
You now want need to create a Pull Request for to offer a fix or a new feature . Pull Request are not fixed in time. I you change your history the PR will be impacted.to be integrated into the main repository.
A A PR will be reviewed by committers and they may ask you additional work.To easy your work it's better changes. This is an asynchronous process and it helps to create a branches branch per feature that you want to submit as Pull Request.
Creating and pointing at at a new branch from master requires 3 those steps:
git checkout master
git pull https://github.com/apache/netbeans.git master
git branch mywork
git checkout mywork
You can may then codeimplement your changes, commit them and push the branch to your forked repository:
git push https://github.com/YOUR_USERNAME/netbeans.git mywork:mywork
You can then use the github UI to create a Pull request from your branch.
(git should print a direct link to the new PR page if this command succeeds, the github page of your fork should now also have a big green create PR button on top)
PR guide on github: https://help.github.com/articles/creating-a-pull-request/
You will need an ICLA (Individual Contributor License Agreement) for important modifications.
Commit message related to JIRA issue must start with [NETBEANS-<issue number>]
Note: If you are a committer, please read the CI section of the reviewer guide before pressing the create PR button.
Squashing commits on a Pull Request
Before submitting your Pull Request it should ideally consist of a single commit only. Consider you've done the following on your branch:
# | Commit |
---|---|
X | [NETBEANS-xxx] Improved YAML lexer Improved ability for night vision and |
Y | Oops, forgot to include lic file |
Z | Javadoc update - corrected spelling |
If the PR is merged into master as-is then all these commits will be in the master too, forever. Therefore, in this example, all three commits should be squashed into one so that only X is left.
<How to squash guide here>https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
After submission (and certainly after someone starts reviewing the PR) you shouldn't touch the PR's history .
Keeping your own fork in sync with Apache GitHub repo
<to be described : possible link we can use: https://help.github.com/articles/syncing-a-fork/>
git fetch upstream
git checkout master
git merge upstream/master
Tip: Syncing your fork only updates your local copy of the repository. To update your fork on GitHub, you must push your changes
(unless instructed to do so).
(Some larger PRs might be merged with multiple commits, but those are usually an exception to the rule. In any case make sure every individual commit is buildable and testable to avoid problems during git bisections and bugfixing)
...
Updating a stale Pull Request
Over time your Pull Request is likely to go stale. A "stale" pull request is one that is no longer up to date with the main line of development, and it needs to be updated before it can be merged into the project. This typically happens because there's meanwhile been changes in main branch on the same files that are included in the PR, thus resulting in a merge conflict.<to be described : possible link we can use:
First, make sure you are in the right branch
git checkout mywork
warning: this will rewrite PR history and move your PRs on top of a freshly updated branch, do this only if reviewers are ok with it:
warning: this might result in conflicts if the same files changed in the main repo as the files you were working on
git pull --rebase --autostash https://github.com/apache/netbeans.git master
Make sure everything is fine with git status / git log and that no conflicts need to be resolved. Resolving conflicts takes practice (e.g using a sandbox repo) and won't be explained here.
Finally, force push the changes into your PR branch of your fork:
git push -f https://github.com
...
/YOUR_USERNAME/netbeans.git mywork:mywork
How to repair commits which have the wrong name/email in the header
if you already made a commit without configuring git properly, you can amend the last commit with:
git commit --amend --author="John Doe <john@doe.org>" --no-edit
Optional: Keeping your own fork in sync with Apache GitHub repo
switch to your local master branch and download latest changes from the main NetBeans repo
git checkout master
git pull https://github.com/apache/netbeans.git master
git push https://github.com/YOUR_USERNAME/netbeans.git master
Note: Please make sure that you use CI resources responsibly. If you don't need github actions on your fork, don't enabled them. (they are off by default in fresh forks)
Good commit messages
A commit should:
- describe what it is in one short sentence,
- followed by an empty line,
- followed by more details describing the change (often in bullet points "- blabla")
Especially the first line should not exceed 72 characters. (NB will mark this as red line in the commit dialog)
Multiple commit authors
A commit has one author and one committer (which may or may not be the same person). There is however a commit message convention which allows to attribute co-authors simply by appending them to the end of the commit message:
example message:
Fixed NPE in Foo#bar.
- details
- details
- details
Co-authored-by: NAME <NAME@EXAMPLE.COM> Co-authored-by: ANOTHER-NAME <ANOTHER-NAME@EXAMPLE.COM>"
Contribution Guidelines
- Before starting to code, it is a good practice to discuss it in the developer mailing list (Mailing lists), giving the reason for submitting your pull request so that it is clear and more experienced members can suggest appropriate solutions/ideas.
- All commits must include the author's full name and email address. For important modifications you will need to submit an Individual Contributor License Agreement (ICLA) .
- All new files must include the Apache Software Foundation license header. See any NetBeans source code in case of doubt.
- All commits must contain a meaningful commit message.
- A meaningful commit message holds in the first line a summary of the commit and in the body (beginning on the third-line) an explanation of what was changed and why it was done.
- Remember that in the future this commit message is most probably the only source of information why a change was committed to the code base.
- If the commit fixes a reported issue, the summary line should hold the issue number and title
"[NETBEANS-XXX] Maven pom.xml file corrupted after inserting dependecies"
for example.
- A Pull Request can consist of multiple commits. These commits should group the changes into meaningful entities. Fixup commits should be squashed into the base commit they fix.
- For contributors: Be prepared to be asked questions about your PR.
- A reviewer might have questions and you should be able to answer why you did a fix in a certain way and why it is safe and appropriate.
- Remember that the review sometimes takes as long, as creating a patch in the first place.
- Good commit messages help as they anticipate questions.
- For reviewers: Keep in mind that the contributor wants to fix a problem and has put effort into it. So be polite and focused.
- Don't change code that is correct and works.
- Consider a simple loop. In many cases you can switch between for-loop, for-each-loop and stream construct. All are valid solutions, don't change the code if it is not broken.
- An improvement is a different case. For example a try-with-resource construct is in general more correct than the try-finally construct which many developers fail to implement correctly.
- Constructs leading to warnings from the
javac
are also good candidates for simple fixes.
- Run unit tests and, if you introduce new feature/fixes, add unit tests. So before you start your work, check that unit tests for the module you are working on run correctly and after you are done keep doing.
- If unit tests fail, fixing these would be a good addition to the code base (it would be good to use a separate commit for this)
- Keep your pull requests up-to-date. When the PR can't be merged directly (it can happen that changes are introduced into the code base, that conflict with your PR) you should then update it accordingly.
- Follow the coding conventions of the file. Your code should match that style and not stand out. For new files, please follow the code conventions for the NetBeans code base: https://netbeans.org/community/guidelines/code-conventions.html
- Try to keep the code readable, maintainable, easy to debug and performant.
And remember: These are guidelines, not laws
PR Coding Best Practices
When submitting a PR keep in mind these principles:
- Please submit an issue to JIRA (https://issues.apache.org/jira/projects/NETBEANS/summary) explaining your setup (platform, tc.), why your PR is needed and a case to reproduce, if possible.
- In the code, try to be concise and right to the point. Just modify whatever needs to be modified to solve the issue at hand.
- If your code is complex:
- submit a unit test as well.
- add comments for the most complex parts.
- Try to keep the code readable, maintainable, easy to debug and performant.
- When submitting a PR start your commit message with NETBEANS-XYZ, where XYZ is the issue number. By doing so your PR will be automatically associated to the JIRA issue.
- After submitting a PR, NetBeans committers will review it for approval before being finally merged. This may take some time, so please be patient.
Streaming API considerations
Avoid using unnecessary streaming API constructs. For instance, this code:
Code Block | ||
---|---|---|
| ||
for (Transaction commit : SPIAccessor.DEFAULT.getCommits(bag)) {
SPIAccessor.DEFAULT.check(commit, true);
} |
Is perfectly readable and performant, but it could be transformed to the Streaming API equivalent:
Code Block | ||
---|---|---|
| ||
SPIAccessor.DEFAULT.getCommits(bag).forEach((commit) -> SPIAccessor.DEFAULT.check(commit, true)); |
But the resulting code will be slower and more difficult to debug, so the change does not really add more value to the source code.
When using the Streaming API always try to write readable constructs and make the code easy to debug (by splitting complex streaming API constructs in multiple lines, for instance).
Keep explicity typing
Try not to remove typing when using generics with the diamond operator. So, for instance, you could replace
Code Block | ||
---|---|---|
| ||
ArrayList<JCVariableDecl> fieldGroup = null;
// lots of lines here...
fieldGroup = new ArrayList<JCVariableDecl>(); |
With
Code Block | ||
---|---|---|
| ||
ArrayList<JCVariableDecl> fieldGroup = null;
// lots of lines here...
fieldGroup = new ArrayList<>(); |
This is, you could use the diamond operator in the assignment, but this will result in a less readable code. Try to keep explicit typing visible as much as possible, pondering always readability over syntax sugar.
Internationalization
If the source code might be translated to other languages then please add the comment `// NOI18N` to the string literals that must not be translated. Strings to be translated should be properly added to resource bundles.
If the source code is not to be translated then avoid adding `// NOI18N` comments to the string literals, ; this just adds clutter and makes the code less readable.