Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify (and possibly improve) issue states. #6655

Closed
marmarek opened this issue May 31, 2021 · 9 comments
Closed

Clarify (and possibly improve) issue states. #6655

marmarek opened this issue May 31, 2021 · 9 comments
Labels
C: other P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. project management This issue pertains to the management of the Qubes OS Project. T: task Type: task. An action item that is neither a bug nor an enhancement.
Milestone

Comments

@marmarek
Copy link
Member

#6321 (comment)

In general, is it expected that an issue with the "Release 4.0 updates" milestone is closed without updating R4.0?

In fact, yes. Issue states in github are quite limited (just "open"/"closed"), so it doesn't allow to express all the possible states:

  1. not fixed yet
  2. fix developed, but not committed yet (PR open)
  3. fix committed (PR merged), but update not pushed anywhere yet
  4. update pushed to testing repository for the most recent development version (R4.1 now)
  5. update pushed to stable repository for the most recent development version (R4.1 now)
  6. update backported to relevant stable version(s) (R4.0 now), and pushed to the testing repository
  7. update pushed to the stable repository of stable version(s)

Currently, we close issue at the step 3. Then, when updates gets released, issue automatically get appropriate labels:

  • r4.1-*-cur-test
  • r4.1-*-stable
  • r4.0-*-cur-test
  • r4.0-*-stable

Based on those labels, it's possible to select issues waiting for step 6 - see https://github.com/QubesOS/qubes-issues#issues-by-release.

We choose to close issues at step 3 mostly because github provides automation for it for free (just mention "Fixes QubesOS/qubes-issues#...` in the commit message). If we decide to close issue at a different stage, we'd need to write automation for that ourselves (but it isn't much work).

This indeed may be confusing for some (issue is closed, but fix is not yet available for some setups). But given the number of states a fix can be in, and just 2 states in github, there will also be some discrepancies. The current workflow is also friendly for developers - when issue is "open" it should be a thing to work on.

At the very least, we should have this documented. But maybe we could improve it somehow?

@marmarek marmarek added T: task Type: task. An action item that is neither a bug nor an enhancement. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. labels May 31, 2021
@marmarek marmarek added this to the Ongoing milestone May 31, 2021
@andrewdavidwong
Copy link
Member

#6321 (comment)

In general, is it expected that an issue with the "Release 4.0 updates" milestone is closed without updating R4.0?

In fact, yes.

Even though I answered "no," I don't think we're in disagreement. The question is ambiguous. I chose to interpret it as asking whether it's expected that an issue with the "Release 4.0 updates" milestone would be closed without ever updating R4.0. In other words, if such an issue gets closed, but no 4.0 update ever drops for it, is that a mistake? I believe it would be, so I answered "no." Due to the ambiguity of the question, I think you and I are both correct.*

I chose to interpret the question this way because it was asked by @mattmccutchen, who (I believe) is familiar enough with our issue tracker to have observed that there is generally a time lag between issue closure and update availability. I figured he wasn't expecting that a 4.0 update would immediately be available as soon as #6321 was closed. Rather, I figured he was wondering why there was still no 4.0 update five months after it was closed. So far, there have only been 4.1 updates for it, the last one on Mar 16, with no indication of any 4.0 updates. This suggested to me that either the milestone was wrong (should have been 4.1 instead) or that the 4.0 updates were forgotten, since usually the time lag is not that long. Are you saying that, in this case, the time lag just is that long, and 4.0 updates are still forthcoming for #6321?

Issue states in github are quite limited (just "open"/"closed"), so it doesn't allow to express all the possible states:

  1. not fixed yet
  2. fix developed, but not committed yet (PR open)
  3. fix committed (PR merged), but update not pushed anywhere yet
  4. update pushed to testing repository for the most recent development version (R4.1 now)
  5. update pushed to stable repository for the most recent development version (R4.1 now)
  6. update backported to relevant stable version(s) (R4.0 now), and pushed to the testing repository
  7. update pushed to the stable repository of stable version(s)

Currently, we close issue at the step 3. Then, when updates gets released, issue automatically get appropriate labels:

  • r4.1-*-cur-test
  • r4.1-*-stable
  • r4.0-*-cur-test
  • r4.0-*-stable

Based on those labels, it's possible to select issues waiting for step 6 - see https://github.com/QubesOS/qubes-issues#issues-by-release.

We choose to close issues at step 3 mostly because github provides automation for it for free (just mention "Fixes QubesOS/qubes-issues#...` in the commit message). If we decide to close issue at a different stage, we'd need to write automation for that ourselves (but it isn't much work).

This indeed may be confusing for some (issue is closed, but fix is not yet available for some setups). But given the number of states a fix can be in, and just 2 states in github, there will also be some discrepancies. The current workflow is also friendly for developers - when issue is "open" it should be a thing to work on.

At the very least, we should have this documented. But maybe we could improve it somehow?

This description matches my understanding of our system. We're on the same page.

I think it's fine as is, but it would be more intuitive for others (users and contributors unfamiliar with the current system) if issues were closed at step 7 instead. If you think that's easy to implement and wouldn't be disruptive to developer work, then it might be a worthwhile improvement. It would probably also be simpler to communicate ("issues are closed when updates are in stable" vs. the entire explanation you just wrote above).


* There is another way I could be wrong, namely if we had a policy of sometimes closing issues on the 4.0 milestone by fixing them in 4.1+ (but not in 4.0) without altering the 4.0 milestone. In fact, I seem to recall that we may have done this at least a few times, but probably without thinking through the implications and certainly without having a "policy" of doing so. We should probably decide whether the appropriate course of action, in such cases, is to change the milestone to the release receiving the actual fix or to do something else.

@andrewdavidwong andrewdavidwong added C: other project management This issue pertains to the management of the Qubes OS Project. labels May 31, 2021
@mattmccutchen
Copy link

Thanks Marek for filing this issue and Andrew for adding your point of view.

Andrew is basically right about my position. I understand that the Qubes project has a ton of things that need to be done and very limited staff time; I tried to help a bit with that by donating some money a few months ago, and I'm very grateful that you're doing as much as you are. But I'd think that backporting already developed fixes to the version of Qubes OS that most(?) users are actually using would typically be more valuable (and thus a higher priority) than fixing more issues, especially for trivial backports like #6321. I even wonder if it would make sense to make a habit of developing the R4.1 and R4.0 updates at the same time in cases such as #6321 where it seems unlikely that a problem would be detected in R4.1 that would result in work on the R4.0 backport having been wasted. I'd be happy to help with backports if I can, but I imagine it's more work for Marek to review the accuracy of a backport from me than to just do it himself.

Whatever priorities and tracking mechanism you all decide on, better documenting it would be greatly appreciated. I know that with your limited time, you need users to "meet you halfway" on many of these things by doing research themselves, but it's good to improve the documentation when we can. Maybe I can submit a PR for that once you make a decision. Would the "reporting bugs" page be a good place for the information? That's the main place I looked for a policy on backports before asking. I certainly don't think the https://github.com/QubesOS/qubes-issues home page (which currently has the link to the "pending backports" query that Marek mentioned) is obvious enough to most users.

@andrewdavidwong
Copy link
Member

I understand that the Qubes project has a ton of things that need to be done and very limited staff time; I tried to help a bit with that by donating some money a few months ago, and I'm very grateful that you're doing as much as you are.

Thank you for your understanding and support!

Would the "reporting bugs" page be a good place for the information?

Yes, this is the page where we'll want to document the policy governing when issues with certain milestones are closed, once it's decided.

@marmarek
Copy link
Member Author

marmarek commented Jun 1, 2021

I think it's fine as is, but it would be more intuitive for others (users and contributors unfamiliar with the current system) if issues were closed at step 7 instead. If you think that's easy to implement and wouldn't be disruptive to developer work, then it might be a worthwhile improvement. It would probably also be simpler to communicate ("issues are closed when updates are in stable" vs. the entire explanation you just wrote above).

Hmm, in fact it may not be that simple change. To work reliably, it would require disabling github's automatic closing issues with special words in commit messages (but also - we do want to keep mentioning which commit fixes which issue).
I've reviewed how some other projects handle this (example issues in parenthesis):

I realize the sample size is small, but also I haven't found a single project that closes issues (on github specifically) only when the change gets released.

In our case, when the change is released, we already have a comment from builder-github bot (and also labels are updated). So, it should should be clear when it happens. I think there are two states when it isn't clear:

  • fix merged, but not released anywhere
  • fix released for R4.1, but backporte to R4.0 is pending (or whatever current versions are)

Maybe we can have some extra (human readable) labels, instead of opaque filter(*) ? Something like "backport-pending", or "release-pending"? We can probably pretty easily automate adding/removing them.

(*) I've just found the filter is not perfect - it picks up issues as "pending backport" when the change in R4.1 is relevant to both dom0 and VM, but in R4.0 to VM only (and was already backported). In this case r4.0-dom0-cur-test (or r4.0-dom0-stable) label won't be added, even if the change is fully backported.

@andrewdavidwong
Copy link
Member

Hmm, in fact it may not be that simple change. To work reliably, it would require disabling github's automatic closing issues with special words in commit messages (but also - we do want to keep mentioning which commit fixes which issue).
I've reviewed how some other projects handle this (example issues in parenthesis):

  • pylint - no info on release at all (PyCQA/pylint#4518 - fixed but not released, PyCQA/pylint#4065 - fixed and released)
  • salt - unsure, but I don't see clear info - there are both milestones and labels, but I think they are about which versions are affected, not which are patched (saltstack/salt#60160 - fixed but not released, saltstack/salt#58412 - fixed and released)
  • NixOS - no clear info, but also unsure if "fixed but not released" applies there (issue that that has been backported to some stable branch: NixOS/nixpkgs#120928), but they do have that info on matching PRs, for example NixOS/nixpkgs#124408

I realize the sample size is small, but also I haven't found a single project that closes issues (on github specifically) only when the change gets released.

Ah, it sounds like we're already doing more than most other projects, then.

In our case, when the change is released, we already have a comment from builder-github bot (and also labels are updated). So, it should should be clear when it happens. I think there are two states when it isn't clear:

  • fix merged, but not released anywhere
  • fix released for R4.1, but backporte to R4.0 is pending (or whatever current versions are)

Maybe we can have some extra (human readable) labels, instead of opaque filter(*) ? Something like "backport-pending", or "release-pending"? We can probably pretty easily automate adding/removing them.

Yes, that's a great idea. However, I think we should also have automated comments for these in addition to the labels, since many people seem to ignore even the clear human-readable labels. Also, I don't think changing the labels triggers an email notification, whereas a comment does, so this could be important for people who primarily use email for interaction or archiving.

(*) I've just found the filter is not perfect - it picks up issues as "pending backport" when the change in R4.1 is relevant to both dom0 and VM, but in R4.0 to VM only (and was already backported). In this case r4.0-dom0-cur-test (or r4.0-dom0-stable) label won't be added, even if the change is fully backported.

We could always get more fine-grained with our label system, but I don't know whether the additional complexity would be worth it. I think the deciding factor should be whether (and to what extent) it supports your work and the work of other developers.

Making the developer workflow transparent to outside observers has value, but it's not as important as the work itself and should never impede it.

@marmarek
Copy link
Member Author

marmarek commented Jun 1, 2021

However, I think we should also have automated comments for these in addition to the labels, since many people seem to ignore even the clear human-readable labels.

Labels can have also tooltips and they also show up in changes history (not only this right pane).

Also, I don't think changing the labels triggers an email notification, whereas a comment does

I kind of prefer label here for this very reason. Label like "backport pending" is IMO more useful as an issue state (if someone finds such issue, to see where is it in the process of hitting R4.0), than an event (whether issue is a backport candidate, should be clear from the very beginning - based on its milestone and affected version). On the other hand, when related package is uploaded to a repository (an event, state change), we already have a comment about it.

We could always get more fine-grained with our label system, but I don't know whether the additional complexity would be worth it. I think the deciding factor should be whether (and to what extent) it supports your work and the work of other developers.

Given the corner case mentioned earlier, I think an explicit "backport pending" label has an extra value: it can be manually adjusted if the automatic assignment somehow fails one way or another. With the current system, I need to handle such cases manually (either by ignoring such issues each time, or by using other place to note things to backport); or by adjusting issues metadata in a confusing way (like changing the labels manually, to values not really matching repository state...).

@andrewdavidwong
Copy link
Member

However, I think we should also have automated comments for these in addition to the labels, since many people seem to ignore even the clear human-readable labels.

Labels can have also tooltips and they also show up in changes history (not only this right pane).

Yes, I know (and have added many tooltips to our labels), but that doesn't stop people from ignoring them. 🙂

Also, I don't think changing the labels triggers an email notification, whereas a comment does

I kind of prefer label here for this very reason. Label like "backport pending" is IMO more useful as an issue state (if someone finds such issue, to see where is it in the process of hitting R4.0), than an event (whether issue is a backport candidate, should be clear from the very beginning - based on its milestone and affected version). On the other hand, when related package is uploaded to a repository (an event, state change), we already have a comment about it.

Ah, ok. Fair enough then.

We could always get more fine-grained with our label system, but I don't know whether the additional complexity would be worth it. I think the deciding factor should be whether (and to what extent) it supports your work and the work of other developers.

Given the corner case mentioned earlier, I think an explicit "backport pending" label has an extra value: it can be manually adjusted if the automatic assignment somehow fails one way or another. With the current system, I need to handle such cases manually (either by ignoring such issues each time, or by using other place to note things to backport); or by adjusting issues metadata in a confusing way (like changing the labels manually, to values not really matching repository state...).

Sounds worthwhile to have this new label, then.

@andrewdavidwong
Copy link
Member

Created the backport pending label with this description:

The fix has been released for the testing release but is pending backport to the stable release.

(Please feel free to improve the description.)

@andrewdavidwong
Copy link
Member

I've revamped the Issue Tracking page (previously known as "How to Report Bugs and Other Issues") and added a Backports section that documents what we discussed here.

@marmarek, if you need to do other things on your end (e.g., automate application of the backport pending label) and would like to use this issue for that, please feel free to reopen.

Also, if what I've done is not what was intended or is not sufficient to close this issue, please reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: other P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. project management This issue pertains to the management of the Qubes OS Project. T: task Type: task. An action item that is neither a bug nor an enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants