You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardexpand all lines: README.md
+2-1
Original file line number
Diff line number
Diff line change
@@ -16,6 +16,8 @@ To engage in the development of the Developers' Guide itself, create a private f
16
16
17
17
The OpenJDK Developers' Guide is written in American English. The Guide is meant to be an informal document. This is reflected in the language used. For instance, contractions are commonly used.
18
18
19
+
When choosing what words to use, keep in mind that a large portion of the audience isn't native English speakers. Always try to keep the language easy to understand and avoid niche words and local terminology if possible. That said, the language must also be precise to avoid mixing up different concepts. For instance the git term `push` is only used when talking about pushes to a private clone of a Project repository. When talking about the Project repositories themselves we use the Skara term `integrate` since developers can't push directly to these repositories.
20
+
19
21
### Keywords and links
20
22
21
23
All terms defined in the OpenJDK Bylaws are considered keywords, and should be capitalized. Company and organization names are also considered keywords and follow the same rules. A keyword should be linked to the definition of that term if the keyword is referring to the defined term. Additional words that build a noun phrase should be included in the link.
@@ -54,7 +56,6 @@ The Developers' Guide is continuously updated and there are several parts still
54
56
* Text about adding an API
55
57
* Text about adding a feature
56
58
* Text about JCK
57
-
* Text about triage, priorities, status
58
59
* List JTReg `@key` conventions for different areas
59
60
* Document best practices around TEST.properties usage. See [PR#30](https://github.com/openjdk/guide/pull/30#issuecomment-714589551)
Copy file name to clipboardexpand all lines: src/guide/backporting.md
+2-2
Original file line number
Diff line number
Diff line change
@@ -93,7 +93,7 @@ In this example a fix was integrated to JDK N+1 (the mainline master branch) whi
93
93
#. Reopen the _backport_ issue that was created automatically
94
94
* Use a comment like the following (in the reopen dialog):
95
95
~~~
96
-
Fix was integrated while main issue was targeted to 'N'. Reset the main issue to fixed in 'N+1', reset this issue to fix in 'na' and closed as Not An Issue to avoid confusion.
96
+
This change was integrated while the main issue was targeted to 'N'. The main issue has been reset to fixed in 'N+1', this backport issue has been reset to fix in 'na' and closed as Not An Issue to avoid confusion.
97
97
~~~
98
98
* Change the [Fix Version/s]{.jbs-field} from 'N+1' to 'na'.
99
99
* Close the _backport_ issue as [Not an Issue]{.jbs-value}. Note: [Closed]{.jbs-value}, **not**[Resolved]{.jbs-value}
* Add a comment like the following to the _main_ issue:
112
112
~~~
113
-
Fix was integrated to 'N+1' while this main issue was targeted to 'N'. Reset this issue to fixed in 'N+1' and copied the Robo Duke entry here.
113
+
This change was integrated to 'N+1' while this main issue was targeted to 'N'. This issue has been reset to fixed in 'N+1' and the Robo Duke entry was copied here.
114
114
~~~
115
115
* Reset the _main_ issue [Fix Version/s]{.jbs-field} from 'N' to 'N+1'.
116
116
* Resolve the _main_ issue as [Fixed]{.jbs-value} in build "team" or in build "master" depending on where the fix was integrated - or to an actual build number if the change has already made it to a promoted build (look in the _backport_ issue if you are unsure). Integrations to 'openjdk/jdk' are fixed in build "master" and integrations to other Project repositories are fixed in build "team".
Copy file name to clipboardexpand all lines: src/guide/contributing-to-an-open-jdk-project.md
+1-1
Original file line number
Diff line number
Diff line change
@@ -64,7 +64,7 @@ Socializing your change on the mailing lists also prevents the surprise that wou
64
64
65
65
### 4. Create a tracking issue in JBS
66
66
67
-
Many [OpenJDK Projects](https://openjdk.org/bylaws#project) require a tracking issue to be filed in the [JDK Bug System (JBS)](https://bugs.openjdk.org/) before a change can be pushed. This is the case for instance for the JDK and the JDK Updates Projects. In order to obtain write access to JBS you need to be an [Author](https://openjdk.org/bylaws#author) in an [OpenJDK Project](https://openjdk.org/bylaws#project) (see [Becoming an Author]). For your first changes, ask your sponsor to help you create the issue or file the bug through the [Bug Report Tool](https://bugreport.java.com/).
67
+
Many [OpenJDK Projects](https://openjdk.org/bylaws#project) require a tracking issue to be filed in the [JDK Bug System (JBS)](https://bugs.openjdk.org/) before a change can be integrated. This is the case for instance for the JDK and the JDK Updates Projects. In order to obtain write access to JBS you need to be an [Author](https://openjdk.org/bylaws#author) in an [OpenJDK Project](https://openjdk.org/bylaws#project) (see [Becoming an Author]). For your first changes, ask your sponsor to help you create the issue or file the bug through the [Bug Report Tool](https://bugreport.java.com/).
Copy file name to clipboardexpand all lines: src/guide/hotspot-development.md
+1-1
Original file line number
Diff line number
Diff line change
@@ -1,6 +1,6 @@
1
1
# HotSpot Development
2
2
3
-
See [Working With Pull Requests] for generic guidance and requirements around pushing changes. For the HotSpot codebase there are a few additional requirements:
3
+
See [Working With Pull Requests] for generic guidance and requirements around integrating changes. For the HotSpot codebase there are a few additional requirements:
4
4
5
5
* Your change must have been approved by two reviewers out of which at least one is also a [Reviewer](https://openjdk.org/bylaws#reviewer)
6
6
* Your change must have passed through HotSpot tier 1 testing with zero failures (See tier1 definition in `test/hotspot/jtreg/TEST.groups`.)
Copy file name to clipboardexpand all lines: src/guide/introduction.md
+3-3
Original file line number
Diff line number
Diff line change
@@ -35,7 +35,7 @@ Note that when a new [Project](https://openjdk.org/bylaws#project) is created an
35
35
36
36
### Becoming an Author
37
37
38
-
Becoming an [Author](https://openjdk.org/bylaws#author) is the first step. To achieve this you need to contribute two changes to the [Project](https://openjdk.org/bylaws#project) in which you wish to become an [Author](https://openjdk.org/bylaws#author). Once your changes are pushed into the code base and has been vetted enough to determine that the changes were indeed good changes you can go ahead and send an email to the Project lead of that particular [Project](https://openjdk.org/bylaws#project) and ask to be added as an [Author](https://openjdk.org/bylaws#author). Note that "the [Project](https://openjdk.org/bylaws#project)" is not OpenJDK, but rather the specific [development Project](https://openjdk.org/bylaws#project) where you did your contributions (e.g. "JDK", "JDK Updates", "Amber", etc). The [OpenJDK Project description](https://openjdk.org/projects/#project-author) has a template for such an email. In short the email should contain your name, the Project name, your email address, and GitHub links to your changes. In response to your email you will get a time-limited invite which you should fill out.
38
+
Becoming an [Author](https://openjdk.org/bylaws#author) is the first step. To achieve this you need to contribute two changes to the [Project](https://openjdk.org/bylaws#project) in which you wish to become an [Author](https://openjdk.org/bylaws#author). Once your changes are integrated into the code base and has been vetted enough to determine that the changes were indeed good changes you can go ahead and send an email to the Project lead of that particular [Project](https://openjdk.org/bylaws#project) and ask to be added as an [Author](https://openjdk.org/bylaws#author). Note that "the [Project](https://openjdk.org/bylaws#project)" is not OpenJDK, but rather the specific [development Project](https://openjdk.org/bylaws#project) where you did your contributions (e.g. "JDK", "JDK Updates", "Amber", etc). The [OpenJDK Project description](https://openjdk.org/projects/#project-author) has a template for such an email. In short the email should contain your name, the Project name, your email address, and GitHub links to your changes. In response to your email you will get a time-limited invite which you should fill out.
39
39
40
40
To see who the Project lead is for your [Project](https://openjdk.org/bylaws#project), see the [OpenJDK Census](https://openjdk.org/census). The [Census](https://openjdk.org/census) unfortunately doesn't provide email addresses for people, but assuming you have been active on the Project mailing list (since you are applying for [Author](https://openjdk.org/bylaws#author) after all), you should be able to find the lead's email address in your local email archive, or ask your [Sponsor](https://openjdk.org/bylaws#sponsor).
41
41
@@ -45,11 +45,11 @@ The rules of any particular [Project](https://openjdk.org/bylaws#project) may ha
45
45
46
46
### Becoming a Committer
47
47
48
-
To become a [Committer](https://openjdk.org/bylaws#committer) you should show that you intend to actively contribute to the [Project](https://openjdk.org/bylaws#project) and that you can produce non-trivial changes that are accepted for inclusion into the Project code base. The number eight has been seen as a formal lower limit on the number of changes, but since the changes must be non-trivial, or "significant" as the [OpenJDK Project description](https://openjdk.org/projects/) says, and the definition of significant is subjective, the general recommendation is to wait with a [Committer](https://openjdk.org/bylaws#committer) nomination until there's at least 10-12 changes pushed to have some margin for different interpretations of "significant". In practice though, we have seen several examples where the number of significant changes hasn't been the dominating factor in a [Committer](https://openjdk.org/bylaws#committer) vote. A [Contributor's](https://openjdk.org/bylaws#contributor) work in another [OpenJDK Project](https://openjdk.org/bylaws#project) may also be relevant for the vote. What the vote should ultimately test is the [Contributor's](https://openjdk.org/bylaws#contributor) commitment to the [OpenJDK Project](https://openjdk.org/bylaws#project) for which the vote applies - is it believed that the person is dedicated and willing to spend time and effort on the [Project](https://openjdk.org/bylaws#project)? Is the person believed to be a good citizen of the [Project](https://openjdk.org/bylaws#project)? It's always a good idea to seek the advice of a [Sponsor](https://openjdk.org/bylaws#sponsor) who can guide you through the process to becoming a [Committer](https://openjdk.org/bylaws#committer) - you will need one to run the Committer vote anyway. They will probably also have a better idea of what constitutes a "significant" change.
48
+
To become a [Committer](https://openjdk.org/bylaws#committer) you should show that you intend to actively contribute to the [Project](https://openjdk.org/bylaws#project) and that you can produce non-trivial changes that are accepted for inclusion into the Project code base. The number eight has been seen as a formal lower limit on the number of changes, but since the changes must be non-trivial, or "significant" as the [OpenJDK Project description](https://openjdk.org/projects/) says, and the definition of significant is subjective, the general recommendation is to wait with a [Committer](https://openjdk.org/bylaws#committer) nomination until there's at least 10-12 changes integrated to have some margin for different interpretations of "significant". In practice though, we have seen several examples where the number of significant changes hasn't been the dominating factor in a [Committer](https://openjdk.org/bylaws#committer) vote. A [Contributor's](https://openjdk.org/bylaws#contributor) work in another [OpenJDK Project](https://openjdk.org/bylaws#project) may also be relevant for the vote. What the vote should ultimately test is the [Contributor's](https://openjdk.org/bylaws#contributor) commitment to the [OpenJDK Project](https://openjdk.org/bylaws#project) for which the vote applies - is it believed that the person is dedicated and willing to spend time and effort on the [Project](https://openjdk.org/bylaws#project)? Is the person believed to be a good citizen of the [Project](https://openjdk.org/bylaws#project)? It's always a good idea to seek the advice of a [Sponsor](https://openjdk.org/bylaws#sponsor) who can guide you through the process to becoming a [Committer](https://openjdk.org/bylaws#committer) - you will need one to run the Committer vote anyway. They will probably also have a better idea of what constitutes a "significant" change.
49
49
50
50
Once you have the required changes, a [Committer](https://openjdk.org/bylaws#committer) in the [Project](https://openjdk.org/bylaws#project) can start a vote by sending an email proposing that you should become a [Committer](https://openjdk.org/bylaws#committer). The email should follow the template found in the [OpenJDK Project description](https://openjdk.org/projects/#project-committer).
51
51
52
-
A [Committer](https://openjdk.org/bylaws#committer) is allowed to push changes without the aid of a [Sponsor](https://openjdk.org/bylaws#sponsor). A [Committer](https://openjdk.org/bylaws#committer) is also allowed to nominate other non-Committers to become [Committers](https://openjdk.org/bylaws#committer) in the [Project](https://openjdk.org/bylaws#project).
52
+
A [Committer](https://openjdk.org/bylaws#committer) is allowed to integrate changes without the aid of a [Sponsor](https://openjdk.org/bylaws#sponsor). A [Committer](https://openjdk.org/bylaws#committer) is also allowed to nominate other non-Committers to become [Committers](https://openjdk.org/bylaws#committer) in the [Project](https://openjdk.org/bylaws#project).
Copy file name to clipboardexpand all lines: src/guide/jbs-jdk-bug-system.md
+5-5
Original file line number
Diff line number
Diff line change
@@ -279,7 +279,7 @@ The [Fix Version/s]{.jbs-field} field should indicate when an issue was fixed. T
279
279
|[Closed]{.jbs-value}<br />[External]{.jbs-value} | Use where the issue is due to a problem in a Java library (not part of the OpenJDK code base), an IDE or other external tool etc. Where known, it's good to provide a link to the site where the issue should be reported. |
280
280
|[Closed]{.jbs-value}<br />[Not an Issue]{.jbs-value} | Use when the behavior is expected and valid (cf. [Won't Fix]{.jbs-value}) and the reporter perhaps has misunderstood what the behavior should be. |
281
281
|[Closed]{.jbs-value}<br />[Migrated]{.jbs-value} | Used rarely, but can be seen when issues are transferred into another project by opening up a separate issue in that project, with the issue in the original project being [Closed]{.jbs-value}. |
282
-
|[Resolved]{.jbs-value}/[Closed]{.jbs-value}<br />[Delivered]{.jbs-value} | Used to close out issues where a change to the code isn't required, common examples are Tasks, Release Notes, and umbrella issues for larger changes. |
282
+
|[Resolved]{.jbs-value}/[Closed]{.jbs-value}<br />[Delivered]{.jbs-value} | Used to close out issues where a change to the code isn't required, common examples are Tasks, Release Notes, and umbrella issues for larger changes. Note that the verification step is always needed to move an issue from [Resolved]{.jbs-value} to [Closed]{.jbs-value}. If your issue doesn't need verification (e.g. it's an umbrella where each sub-task is verified as a separate fix) then please move the issue directly to [Closed]{.jbs-value} without going through [Resolved]{.jbs-value}. |
283
283
|[Closed]{.jbs-value}<br />[Withdrawn]{.jbs-value} |[Withdrawn]{.jbs-value} is essentially the partner state to [Delivered]{.jbs-value} for issues that would not have resulted in a fix to the repo, and also part of the [CSR](https://wiki.openjdk.org/display/csr/Main) and [Release Note](#release-notes) process. |
284
284
|[Closed]{.jbs-value}<br />[Approved]{.jbs-value} | Used as part of the [CSR process](https://wiki.openjdk.org/display/csr/Main). |
285
285
| Challenge States|[Exclude [Challenge]]{.jbs-value}, [Alternative Tests [Challenge]]{.jbs-value}, and [Reject [Challenge]]{.jbs-value} are only relevant within the context of the JCK Project. |
@@ -294,10 +294,6 @@ When an issue is closed as [Won't Fix]{.jbs-value}, do not remove the [Fix Versi
294
294
The fix version [na]{.jbs-value} should only be used on backport issues that is created by mistake. See [How to fix an incorrect backport creation in JBS].
295
295
:::
296
296
297
-
### Closing issues without knowing what fixed it
298
-
299
-
If it's determined that an issue has been fixed, but it's unknown what change that fixed it, closing as [Fixed]{.jbs-value} is not an option as this requires a changeset in a project repository. [Duplicate]{.jbs-value} is also not an option since this requires a duplicate-link to the issue that fixed it. A common way to handle such cases is to close the issue as [Delivered]{.jbs-value} with the [Fix version/s]{.jbs-value} set to [unknown]{.jbs-value}. Closing an issue as [Cannot Reproduce]{.jbs-value} has also been common practice but is no longer recommended if it's known that the issue has actually been fixed.
300
-
301
297
### Closing issues as duplicates
302
298
303
299
If the same issue is described in another JBS issue then close one against the other as [Closed]{.jbs-value}/[Duplicate]{.jbs-value}. In general the newer issue is closed as a [Duplicate]{.jbs-value} of the older one, but where the newer issue has a clearer description, or more useful, up-to-date comments then doing it the other way round is ok as long as none of them has been [Fixed]{.jbs-value} already. If one of the issues has been [Fixed]{.jbs-value} the other one should be closed as a [Duplicate]{.jbs-value} of the [Fixed]{.jbs-value} issue. There may be other reasons to choose to close one or the other issue as the [Duplicate]{.jbs-value}. As always - use your best judgement to make the end result as good as possible.
@@ -310,6 +306,10 @@ Any issue closed as a [Duplicate]{.jbs-value} **must** have a "Duplicates" link
310
306
Be mindful of labels on issues closed as [Duplicate]{.jbs-value}. Some labels need to be copied over to the duplicating issue, see for instance the [[tck-red-]{.jbs-label}*(Rel)*](#tck-red-rel) label.
311
307
:::
312
308
309
+
### Closing issues without knowing what fixed it
310
+
311
+
If it's determined that an issue has been fixed, but it's unknown what change that fixed it, closing as [Fixed]{.jbs-value} is not an option as this requires a changeset in a project repository. [Duplicate]{.jbs-value} is also not an option since this requires a duplicate-link to the issue that fixed it. A common way to handle such cases is to close the issue as [Delivered]{.jbs-value} with the [Fix version/s]{.jbs-value} set to [unknown]{.jbs-value}. Closing an issue as [Cannot Reproduce]{.jbs-value} has also been common practice but is no longer recommended if it's known that the issue has actually been fixed.
312
+
313
313
### Closing incomplete issues
314
314
315
315
As mentioned above, issues that lack the information needed to investigate the problem are placed in status [Resolved]{.jbs-value} - [Incomplete]{.jbs-value}. Triage teams should monitor incomplete issues in their area and if needed ping the relevant person. If the required information hasn't been obtained within reasonable time (3-4 weeks) the bug should be closed as [Incomplete]{.jbs-value}.
Copy file name to clipboardexpand all lines: src/guide/project-maintenance.md
+4-4
Original file line number
Diff line number
Diff line change
@@ -63,9 +63,9 @@ The commands above will likely run without a hitch up until the final `git merge
63
63
64
64
For complicated merges, see [Sharing the work] below.
65
65
66
-
### Test before push
66
+
### Test before integration
67
67
68
-
Regardless of if you encountered conflicts or not, you should always build and test your merge before pushing it to your Project repository. Testing needs to be done even when there are no textual conflicts as changes like for instance a rename can result in a compile or test error without any conflict. One could argue that `git merge --no-commit` could be used and have logical errors fixed in the merge commit. However, a subsequent "Fix logical merge errors" commit, is in fact more useful, as it clearly shows the Project specific adjustments needed for incoming changes.
68
+
Regardless of if you encountered conflicts or not, you should always build and test your merge before integrating it to your Project repository. Testing needs to be done even when there are no textual conflicts as changes like for instance a rename can result in a compile or test error without any conflict. One could argue that `git merge --no-commit` could be used and have logical errors fixed in the merge commit. However, a subsequent "Fix logical merge errors" commit, is in fact more useful, as it clearly shows the Project specific adjustments needed for incoming changes.
69
69
70
70
It's always okay to have further commits to clean up after a merge. Hiding a large amount of reworking Project code to fit with upstream changes in a single merge commit will make it hard for further errors post integration to be identified.
71
71
@@ -84,7 +84,7 @@ Make sure the PR title starts with "Merge". You may have noticed that when you i
84
84
85
85
It's always a good idea to also include what was merged in the title of the PR. If you for instance is pulling in JDK mainline into your Project repository it's likely (because it's in general a good idea) that you choose some stable EA tag in mainline to merge. Your PR title could then be something like "Merge jdk-21+2".
86
86
87
-
Whether a merge requires a review or not is up to your Project lead to decide. Many [Projects](https://openjdk.org/bylaws#project) don't require this so the GitHub bots will allow you to push the merge as soon as the [GHA](#github-actions)s are done. (They actually allow you to push even before the GHAs are done, but that's in general not a good idea.)
87
+
Whether a merge requires a review or not is up to your Project lead to decide. Many [Projects](https://openjdk.org/bylaws#project) don't require this so the GitHub bots will allow you to integrate the merge as soon as the [GHA](#github-actions)s are done. (They actually allow you to integrate even before the GHAs are done, but that's in general not a good idea.)
88
88
89
89
Once the PR has been integrated, you can clean up your fork and its clone in preparation for the next merge.
90
90
@@ -124,7 +124,7 @@ An alternative to parking a merge with conflicts in place, is to incrementally m
124
124
* Find yourself in trouble, identify which change is causing the issue.
* Merge up to the previous change, commit and push.
127
+
* Merge up to the previous change, commit and integrate.
128
128
* Ask others to continue the merge from the troubled change forward, how far forward is up you of course, either just that troublesome change, or the rest of the merge up to the $TAG.
129
129
* Rinse and repeat: There may appear further conflicts requiring other [Contributors'](https://openjdk.org/bylaws#contributor) help.
Copy file name to clipboardexpand all lines: src/guide/release-notes.md
+2-2
Original file line number
Diff line number
Diff line change
@@ -19,9 +19,9 @@ Writing the release note is the responsibility of the engineer who owns the issu
19
19
20
20
When writing a release note, be prepared for rather picky review comments about grammar, typos, and wording. This is for the sake of the Java community as a whole, as the language of the release note sets the tone for many blogs and news articles. For a widely used product like the JDK, the release notes are often copied verbatim (including typos) and published to highlight news in the release. This means that we need to take extra care to make sure the text in the release note is correct and follows a similar style.
21
21
22
-
The release note itself is written in a [JBS](#jbs---jdk-bug-system) sub-task of the issue that is used to push the change. There are a few steps to follow for the release note to find its way from JBS to the actual release note document.
22
+
The release note itself is written in a [JBS](#jbs---jdk-bug-system) sub-task of the issue that is used to integrate the change. There are a few steps to follow for the release note to find its way from JBS to the actual release note document.
23
23
24
-
#. Create a sub-task (More → Create Sub-Task) for the issue that requires a release note - the main issue, that is, the JBS issue that is used to push the original change, **not** for backports or the CSR (if there is one).
24
+
#. Create a sub-task (More → Create Sub-Task) for the issue that requires a release note - the main issue, that is, the JBS issue that is used to integrate the original change, **not** for backports or the CSR (if there is one).
25
25
26
26
#. For the newly created sub-task, follow these steps:
27
27
* The [Summary]{.jbs-field} should be a one sentence synopsis that is informative (and concise) enough to attract the attention of users, developers, and maintainers who might be impacted by the change. It should succinctly describe what has actually changed, not be the original bug title, nor describe the problem that was being solved. It should read well as a sub-section heading in a document.
Copy file name to clipboardexpand all lines: src/guide/testing-the-jdk.md
+3-3
Original file line number
Diff line number
Diff line change
@@ -12,7 +12,7 @@ In addition to your own Java applications, OpenJDK has support for two test fram
12
12
13
13
This section provides a brief summary of how to get started with testing in OpenJDK. For more information on configuration and how to use the OpenJDK test framework, a.k.a. "run-test framework", see [`doc/testing.md`](https://github.com/openjdk/jdk/blob/master/doc/testing.md).
14
14
15
-
Please note that what's mentioned later in this section, like [GHA](#github-actions) and tier 1 testing, will only run a set of smoke-tests to ensure your change compiles and runs on a variety of platforms. They won't do any targeted testing on the particular code you have changed. You must always make sure your change works as expected before pushing, using targeted testing. In general all changes should come with a regression test, so if you're writing product code you should also be writing test code. Including the new tests (in the right places) in your change will ensure your tests will be run as part of your testing on all platforms and in the future.
15
+
Please note that what's mentioned later in this section, like [GHA](#github-actions) and tier 1 testing, will only run a set of smoke-tests to ensure your change compiles and runs on a variety of platforms. They won't do any targeted testing on the particular code you have changed. You must always make sure your change works as expected before integrating, using targeted testing. In general all changes should come with a regression test, so if you're writing product code you should also be writing test code. Including the new tests (in the right places) in your change will ensure your tests will be run as part of your testing on all platforms and in the future.
16
16
17
17
A few key items to think about when writing a regression test:
18
18
@@ -187,7 +187,7 @@ In the past there used to be a sandbox repository that could be used for testing
187
187
188
188
Sometimes tests break. It could be e.g. due to bugs in the test itself, due to changed functionality in the code that the test is testing, or changes in the environment where the test is executed. While working on a fix, it can be useful to stop the test from being executed in everyone else's testing to reduce noise, especially if the test is expected to fail for more than a day. There are two ways to stop a test from being run in standard test runs: ProblemListing and using the `@ignore` keyword. Removing tests isn't the standard way to remove a failure. A failing test is often a regression and should ideally be handled with high urgency.
189
189
190
-
Remember to remove the JBS id from the ProblemList or the test when the bug is closed. This is especially easy to forget if a bug is closed as [Duplicate]{.jbs-value} or [Won't Fix]{.jbs-value}. jcheck will report an error and prohibit a push if a PR is created for an issue that is found in a ProblemList if the fix doesn't remove the bug ID from the ProblemList.
190
+
Remember to remove the JBS id from the ProblemList or the test when the bug is closed. This is especially easy to forget if a bug is closed as [Duplicate]{.jbs-value} or [Won't Fix]{.jbs-value}. jcheck will report an error and prohibit an integration if a PR is created for an issue that is found in a ProblemList if the fix doesn't remove the bug ID from the ProblemList.
191
191
192
192
### ProblemListing jtreg tests
193
193
@@ -300,7 +300,7 @@ After a failure is handled by excluding a test, the main JBS issue should be re-
300
300
301
301
## Backing out a change
302
302
303
-
If a change causes a regression that can't be fixed within reasonable time, the best way to handle the regression can be to back out the change. Backing out means that the inverse (anti-delta) of the change is pushed to effectively undo the change in the repository. There are two parts to this task, how to do the bookkeeping in JBS, and how to do the actual backout in git or Mercurial.
303
+
If a change causes a regression that can't be fixed within reasonable time, the best way to handle the regression can be to back out the change. Backing out means that the inverse (anti-delta) of the change is integrated to effectively undo the change in the repository. There are two parts to this task, how to do the bookkeeping in JBS, and how to do the actual backout in git or Mercurial.
304
304
305
305
The backout is a regular change and will have to go through the standard code review process, but is considered a [trivial](#trivial) change. The rationale is that a backout is usually urgent in nature and the change itself is automatically generated. In areas where two reviewers are normally required, only one additional [Reviewer](https://openjdk.org/bylaws#reviewer) is required for a backout since the person who is performing the backout also will review the change.
Copy file name to clipboardexpand all lines: src/guide/working-with-pull-requests.md
+7-3
Original file line number
Diff line number
Diff line change
@@ -20,7 +20,7 @@ The timing of your change will also affect the availability of reviewers. The JD
20
20
21
21
## Rebase before creating the PR
22
22
23
-
It's likely that other people have pushed changes to the code base since you created your branch. Make sure to pull the latest changes and rebase your fix on top of that before creating your PR. This is a courtesy issue. Your reviewers shouldn't have to read your patch on top of old code that has since changed. This is hopefully obvious in cases where the upstream code has gone through cleanups or refactorings, and your patch may need similar cleanups in order to even compile. But even in cases where only smaller changes have been done, the reviewers shouldn't have to react to issues like "that line of code was moved last week, why is it back there?".
23
+
It's likely that other people have integrated changes to the code base since you created your branch. Make sure to pull the latest changes and rebase your fix on top of that before creating your PR. This is a courtesy issue. Your reviewers shouldn't have to read your patch on top of old code that has since changed. This is hopefully obvious in cases where the upstream code has gone through cleanups or refactorings, and your patch may need similar cleanups in order to even compile. But even in cases where only smaller changes have been done, the reviewers shouldn't have to react to issues like "that line of code was moved last week, why is it back there?".
24
24
25
25
Most changes are made to the `master` branch of the mainline repository. In these cases, rebase using:
26
26
@@ -56,6 +56,10 @@ If you have an actual reason to create a PR before the change is all done, make
56
56
57
57
There are changes that span across several areas, for example wide spread cleanups or the introduction of a new language feature. Accordingly, the number of lines of code touched can be quite large, which makes it harder to review the entire PR. In such cases, it may make sense to split the change into several PRs, most commonly by grouping them by module or area.
58
58
59
+
#. **Make sure you target the correct branch**
60
+
61
+
Many project repositories have several branches. Make sure your PR targets the correct one. This is of course especially important when not targeting the default branch. At the top of your PR, right below the title, it will say "NNN wants to merge X commit(s) into [branch]". Typical red flags to watch out for are if your PR seems to include commits or changed files that shouldn't be part of your integration. E.g. Seeing the "Start of release updates for JDK N" when backporting something to JDK N-1 is a bad sign.
62
+
59
63
#. **Set a correctly formatted title**
60
64
61
65
The title of the PR should be of the form "`nnnnnnn: Title of JBS issue`" where `nnnnnnn` is the JBS issue id of the main JBS issue that is being fixed, and the `Title of JBS issue` is the exact title of the issue as written in JBS. In fact, the title can be set to _only_ the JBS issue id (`nnnnnnn`) in which case the bot will fetch the title from JBS automatically. If you are creating a backport PR, see [Using the Skara tooling to help with backports] for more details on the title requirements.
@@ -76,7 +80,7 @@ If you have an actual reason to create a PR before the change is all done, make
76
80
77
81
#. **Allow enough time for review**
78
82
79
-
In general all PRs should be open for at least 24 hours to allow for reviewers in all time zones to get a chance to see it. It may actually happen that even 24 hours isn't enough. Take into account weekends, holidays, and vacation times throughout the world and you'll realize that a change that requires more than just a trivial review may have to be open for a while. In some areas [trivial] changes are allowed to be pushed without the 24 hour delay. Ask your reviewers if you think this applies to your change.
83
+
In general all PRs should be open for at least 24 hours to allow for reviewers in all time zones to get a chance to see it. It may actually happen that even 24 hours isn't enough. Take into account weekends, holidays, and vacation times throughout the world and you'll realize that a change that requires more than just a trivial review may have to be open for a while. In some areas [trivial] changes are allowed to be integrated without the 24 hour delay. Ask your reviewers if you think this applies to your change.
80
84
81
85
#. **Get the required reviews**
82
86
@@ -106,7 +110,7 @@ If you have an actual reason to create a PR before the change is all done, make
106
110
107
111
#. **After integration**
108
112
109
-
After you have integrated your change you are expected to stay around in case there are any issues with it. As mentioned above, you are expected to have run all relevant testing on your change before integrating your PR, but regardless of how thorough you test it, things might slip through. After your change has been integrated an automatic pipeline of tests is triggered and your change will be tested on a variety of platforms and in a variety of different modes that the JDK can be executed in. A change that causes failures in this testing may be backed out if a fix can't be provided fast enough, or if the developer isn't responsive when noticed about the failure. Note that this directive should be interpreted as "it's a really bad idea to push a change the last thing you do before bedtime, or the day before going on vacation".
113
+
After you have integrated your change you are expected to stay around in case there are any issues with it. As mentioned above, you are expected to have run all relevant testing on your change before integrating your PR, but regardless of how thorough you test it, things might slip through. After your change has been integrated an automatic pipeline of tests is triggered and your change will be tested on a variety of platforms and in a variety of different modes that the JDK can be executed in. A change that causes failures in this testing may be backed out if a fix can't be provided fast enough, or if the developer isn't responsive when noticed about the failure. Note that this directive should be interpreted as "it's a really bad idea to integrate a change the last thing you do before bedtime, or the day before going on vacation".
0 commit comments