Addition of a causes link type #139
Conversation
|
👋 Welcome back calnan! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
|
|
||
| ‘blocks’ - when a fix is broken down into a number of parts the ‘blocks’ link should be used to ensure they are all fixed before the main issue is considered resolved - see [implementing large changes](#implementing-large-changes] | ||
|
|
||
| ‘relates to’ - there are no rules as to when or why to create a relates link apart from not duplicating an existing “duplicated by”, ‘backported by’, ‘csr for’ or ‘blocked by’ links. In general, you should link any other issue that has a bearing on the situation where you feel the related issue should be reviewed in order to have a better understanding of what is going on |
There was a problem hiding this comment.
I'd also caution against excessive linking with "relates to" - for example, if a test fails you should not link every other issue with which this test has been previously been associated just because the same test was involved. The "relates to" issue should have some significance in relation to the cause and/or fix, for the current issue.
Note that it is often the case that a "relates to" situation is later determined to be a "duplicates" situation, in which case you have to manually delete the "relates to link" after Closing. (I wonder if JBS could automate that?)
src/guide/jbs-jdk-bug-system.md
Outdated
|
|
||
| ‘relates to’ - there are no rules as to when or why to create a relates link apart from not duplicating an existing “duplicated by”, ‘backported by’, ‘csr for’ or ‘blocked by’ links. In general, you should link any other issue that has a bearing on the situation where you feel the related issue should be reviewed in order to have a better understanding of what is going on | ||
|
|
||
| ‘causes’/‘caused by’ - the causes link implies a stronger relationship than relates. If an issue B is filed which can be traced back to the fix for issue A then ‘A causes B’ (or ‘B was caused by A’). A ‘causes’ link would be added to A if after the integration or release of A it is found that additional work needs to be done. This might be that extra work in another area forgotten and needs to be completed or the more common case would be that a fix ‘causes’ a change of behavior (intentional or otherwise). For a regression, if you identify the fix that caused it, add a caused-by link to that issue and set the Introduced in Build or Introduced in Version field. Whenever looking to backport a fix the developer should look for both ‘blocked by’ and ‘causes’ links in order to understand the set of fixes that should be backported. Likewise, if A has already been backported the new causes linked issues will need to be assessed to see if it is important enough to be backported as well. On B make sure to set the [Introduced in Build]{.jbs-field} or [Introduced in Version]{.jbs-field} field. |
There was a problem hiding this comment.
It seems somewhat disproportionate to explain caused by in so much depth compared to the others. The backporting notes really belong elsewhere, not in this definition list. I would suggest a simpler and more succinct definition:
'causes'/'caused-by' - For when the current issue can be directly attributed to a code change that was integrated under another issue. When such a cause is known you can also set the 'Introduced in Version' and 'Introduced in Build' fields.
There was a problem hiding this comment.
I also think it will be common for a "relates" link to turn into a "cause" link, in which case we need general instructions about removing multiple links to the same issue. Ideally JBS would let us edit a link and change its type.
There was a problem hiding this comment.
In some cases a fix can reveal an existing bug, e.g. because it makes an unlikely issue more likely, or because it added more tests which then find an existing bug. From the sound of it, just because a failure started to occur after commit xyz, doesn't necessarily mean that that commit 'causes' the issue? Some explicit guidance on whether to apply this link type in such cases would be useful I think.
… by link, moving most of the text into other relevant sections of the guide
| @@ -29,6 +29,8 @@ If, for instance, there are other changes between the original one and the follo | |||
|
|
|||
| Testing each individual change is more likely to find issues than just testing the single merged change. It's also easier and less error prone to use the `/backport` command on each commit instead of manually cherrypick and deal with the merges etc. | |||
|
|
|||
| Whenever looking to backport a fix the developer should look for both ‘blocked by’ and ‘causes’ links in order to understand the set of fixes that should be backported. Likewise, if A has already been backported the new causes linked issues will need to be assessed to see if it is important enough to be backported as well. | |||
There was a problem hiding this comment.
| Whenever looking to backport a fix the developer should look for both ‘blocked by’ and ‘causes’ links in order to understand the set of fixes that should be backported. Likewise, if A has already been backported the new causes linked issues will need to be assessed to see if it is important enough to be backported as well. | |
| Whenever looking to backport a fix the developer should look for both ‘blocked by’ and ‘causes’ links in order to understand the set of fixes that should be backported. Likewise, if A has already been backported the new ‘causes’ linked issues will need to be assessed to see if it is important enough to be backported as well. |
Maybe even use italics for the relation ship type? “…for both ‘blocked by’ and ‘causes’ links…” and “…the new ‘causes’ linked issues…”
| * Changes that don't affect product code, but are only against the regression test, or problem-listing: [[noreg-self]{.jbs-label}](#noreg-self) | ||
| * Changes that don't affect product code, but are only against documentation: [[noreg-doc]{.jbs-label}](#noreg-doc) | ||
| * Well contained issues that seem to be easy to fix: [[starter]{.jbs-label}](#starter) | ||
| * Enhancements that are pure cleanups: [[cleanup]{.jbs-label}](#cleanup) | ||
| * Project specific issues usually have their own labels as well | ||
| 1. Managing regressions - for a bug (B) where behavior has _incorrectly_ changed from a previous fix (A) sure that the label [[regression]{.jbs-label}](#regression) is added. Once it is known what fix caused the regression a 'caused by' link should be added to 'B' or a causes link to 'A'. A ‘causes’ link would be added to A if after the integration or release of A it is found that additional work needs to be done. This might be that extra work in another area forgotten and needs to be completed or the more common case would be that a fix ‘causes’ a change of behavior (intentional or otherwise). If 'A' has been identifed as well as a caused-by link to that issue and set the [Introduced in Build]{.jbs-field} and [Introduced in Version]{.jbs-field} fields of 'B', based on which release 'A' was fixed in. |
There was a problem hiding this comment.
| 1. Managing regressions - for a bug (B) where behavior has _incorrectly_ changed from a previous fix (A) sure that the label [[regression]{.jbs-label}](#regression) is added. Once it is known what fix caused the regression a 'caused by' link should be added to 'B' or a causes link to 'A'. A ‘causes’ link would be added to A if after the integration or release of A it is found that additional work needs to be done. This might be that extra work in another area forgotten and needs to be completed or the more common case would be that a fix ‘causes’ a change of behavior (intentional or otherwise). If 'A' has been identifed as well as a caused-by link to that issue and set the [Introduced in Build]{.jbs-field} and [Introduced in Version]{.jbs-field} fields of 'B', based on which release 'A' was fixed in. | |
| 1. Managing regressions - for a bug (B) where behavior has _incorrectly_ changed from a previous fix (A) ensure that the label [[regression]{.jbs-label}](#regression) is added. Once it is known what fix caused the regression a ’caused by’ link should be added to 'B' or a ‘causes’ link to 'A'. A ‘causes’ link would be added to A if after the integration or release of A it is found that additional work needs to be done. This might be that extra work in another area forgotten and needs to be completed or the more common case would be that a fix ‘causes’ a change of behavior (intentional or otherwise). If 'A' has been identified as well as a caused-by link to that issue and set the [Introduced in Build]{.jbs-field} and [Introduced in Version]{.jbs-field} fields of 'B', based on which release 'A' was fixed in. |
In “…a ’caused by’ link should be added to 'B' or a ‘causes’ link to 'A'”, the word “causes” needs the single-quotes to refer to the type of the link. (Also for consistency with other instances.)
In “…that a fix ‘causes’ a change of behavior…”, the word “causes” should not have the quotes because the verb is used in its literal meaning rather being a type.
I find it really hard to understand the last two sentences in this paragraph. “This might be that extra work in another area was forgotten…” — is “was” missing here? The part after “or”: if I understand correctly, “the more common case” is a clarification, wrapping it the commas, dashes or parentheses could be easier to understand. However, it it's the most common case, should it be listed first, instead?
In the last sentence, “If 'A' has been identified as well as a caused-by link…” what does “as well as” compare to? Is it just “as”? And the link type should have the single quotes around it and not have the hyphen “…as a ‘caused by’ link…”.
Using italics for marking up the link type (in addition to the single quotes) could clarify where the text refers to a type rather than uses the verbs literally.
| * The [Description]{.jbs-field} usually explains what went wrong and how the failure was found, then there's some investigation and eventually the root cause is found. At this point the [Summary]{.jbs-field} should be updated to correctly describe the bug. The [Description]{.jbs-field} however should remain a description of how the failure was found. | ||
| * The [Affects Version/s]{.jbs-field} should be updated if you in your investigation finds that the issue is older than what is indicated by the current [Affects Version/s]{.jbs-field}. | ||
|
|
||
| ### Linking Issues | ||
|
|
||
| An important aspect of any issue is making clear how it is connected/related to other issues. This can occur at any stage of the issue's lifecycle. For example, as information becomes available that might suggest a cause, or similar issue (relates to); or when a Backport or CSR request is created; or when closing as a duplicate of another issue. |
There was a problem hiding this comment.
| An important aspect of any issue is making clear how it is connected/related to other issues. This can occur at any stage of the issue's lifecycle. For example, as information becomes available that might suggest a cause, or similar issue (relates to); or when a Backport or CSR request is created; or when closing as a duplicate of another issue. | |
| An important aspect of any issue is making clear how it is connected/related to other issues. This can occur at any stage of the issue's lifecycle. For example, as information becomes available that might suggest a cause, or similar issue (relates to). |
This should be enough as an example. The following list presents what link types are available and their meaning.
|
|
||
| There are the following link types: | ||
|
|
||
| ’duplicate of` - Normally set automatically - see [Closing issues as duplicates] for more information |
There was a problem hiding this comment.
| ’duplicate of` - Normally set automatically - see [Closing issues as duplicates] for more information | |
| ‘duplicate of’ - Normally set automatically - see [Closing issues as duplicates] for more information |
Does markdown support a definition list? Should we use HTML <dl> and <dt>, <dd> tags to mark up the list?
|
|
||
| ’duplicate of` - Normally set automatically - see [Closing issues as duplicates] for more information | ||
|
|
||
| ‘backported by’ - Normally set automatically when creating a backport with the “More -> Create Backport” option |
There was a problem hiding this comment.
Or when the backport is integrated?
|
|
||
| An important aspect of any issue is making clear how it is connected/related to other issues. This can occur at any stage of the issue's lifecycle. For example, as information becomes available that might suggest a cause, or similar issue (relates to); or when a Backport or CSR request is created; or when closing as a duplicate of another issue. | ||
|
|
||
| There are the following link types: |
There was a problem hiding this comment.
I suggest marking up the issue type with bold ** if not with <dt> and using a descriptive sentence maybe, for example:
‘duplicate of’ is normally set automatically when another issue is closed as a ‘duplicate’, see [Closing issues as duplicates] for more information.
‘backported by’ is set automatically when a fix is pushed to an older release or when a backport is created manually using the More -> Create Backport option.
The list would be easier to read if the description of all the link types followed the same pattern.
|
|
||
| ‘relates to’ - there are no rules as to when or why to create a relates link apart from not duplicating an existing “duplicated by”, ‘backported by’, ‘csr for’ or ‘blocked by’ links. In general, you should link any other issue that has a bearing on the situation where you feel the related issue should be reviewed in order to have a better understanding of what is going on | ||
|
|
||
| ‘causes’/‘caused by’ - the causes link implies a stronger relationship than relates. If an issue B is filed which can be traced back to the fix for issue A then ‘A causes B’ (or ‘B was caused by A’) |
There was a problem hiding this comment.
| ‘causes’/‘caused by’ - the causes link implies a stronger relationship than relates. If an issue B is filed which can be traced back to the fix for issue A then ‘A causes B’ (or ‘B was caused by A’) | |
| ‘causes’/‘caused by’ - the ‘causes’ link implies a stronger relationship than ‘relates to’. If an issue 'B' is traced back to the fix for issue 'A' then ‘A causes B’ (or ‘B was caused by A’) |
Should it be ‘B is caused by A’?
In the above text about regressions, issue A and B have dumb single quotes around them. This should be consistent here, therefore I added them in the suggestion.
There is a proposal for the addition of a causes/caused by link type. Tied into that there is now a separate section which covers the main link types that we use
Progress
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/guide.git pull/139/head:pull/139$ git checkout pull/139Update a local copy of the PR:
$ git checkout pull/139$ git pull https://git.openjdk.org/guide.git pull/139/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 139View PR using the GUI difftool:
$ git pr show -t 139Using diff file
Download this PR as a diff file:
https://git.openjdk.org/guide/pull/139.diff
Using Webrev
Link to Webrev Comment