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

ISSUE_TEMPLATE: convert into YAML forms #368656

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

SigmaSquadron
Copy link
Contributor

@SigmaSquadron SigmaSquadron commented Dec 27, 2024

GitHub supports Issue Forms, a bespoke way to query users for information when creating issues. This pull request is the culmination of my annoyance at issues like these, as well as many other badly-filled-out issues.

These forms guide the user in a step-by-step journey to filing actionable issues. We should see far less invalid issues, and far more issues that include all of the necessary information to develop a solution to the problem they raise.

Things done

  • No rebuilds, obviously.
  • As this is related to the developer experience on GitHub, there's no need for a changelog entry.
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

This changes the blank issue template to a simple HTML comment telling
users to use the forms instead, while still allowing experienced
contributors to create free-form issues.

Signed-off-by: Fernando Rodrigues <[email protected]>
@github-actions github-actions bot added 6.topic: policy discussion 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions labels Dec 27, 2024
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Dec 27, 2024
@SigmaSquadron
Copy link
Contributor Author

A testing area with all of these forms is available here. Feel free to create test issues to inspect their outputs and layout.

Copy link
Contributor

@pluiedev pluiedev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, just from a glance, the amount of issue templates seem a bit excessive (ten in total) — so much so that it struggles to fit on my screen. The more templates we have, the more users have to think before choosing a category which adds a considerable barrier to entry.

The order in which these are laid out are also quite strange. Backport requests, then bug reports, build failures, requests for docs/modules/packages, tracking issues (which only existing contributors should make, honestly), and update requests. I suspect the reason is that it's laid out in lexicographic order by default, but that's plainly unintuitive. Based on the frequency of these issue types in my experience, IMO the order should be more:

  • Bug report (unify all different categories too ­— we can't expect people to know the difference between a NixOS module bug and a package bug all the time, and Darwin should ideally just be a checkbox)
  • Build failure
  • Request: new package
  • Request: package update — note: we should tell people that diverging forks should count as new packages, while a non-diverging one should be a package update (we should define what does mean, too); they should also check any recent merged PR against master as sometimes people make package update requests only to realize that it's in master but not unstable
  • Request: new NixOS module
  • Request: backport to stable
  • Request: documentation
  • Tracking issue (remove: contributors should make those manually)

I haven't read each template thoroughly yet, but I think I'll have something to say about that too

.github/ISSUE_TEMPLATE/backport_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/backport_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/backport_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/backport_request.yml Outdated Show resolved Hide resolved
> **Before you begin:** Be advised that backports are subject to the [release suitability guidelines](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#changes-acceptable-for-releases). As a rule of thumb, Nixpkgs Stable will almost never recieve breaking changes in the form of major package updates, but minor updates and bugfixes should be backported when possible.
> Please read the rules above carefully before filing this backport request.

Welcome to Nixpkgs. Please replace the **`Backport to Stable: PACKAGENAME OLDVERSION → NEWVERSION`** template above with the correct package name (As seen in the [NixOS Package Search](https://search.nixos.org/packages)), the current version of the package in Nixpkgs Stable and the current version of the package in Nixpkgs Unstable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always felt like the whole UPPERCASE means variable thing had contributed to user confusion. Maybe we should switch to <angle-brackets> instead? (Feedback/debate welcome)

Copy link
Contributor Author

@SigmaSquadron SigmaSquadron Dec 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be quite confusing as can be easily misread as ...

Is the main reason why I kept the UPPERCASE variables. Angle brackets also introduce the confusion of "should I also replace the angle brackets?"

.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/unreproducible_package.yml Outdated Show resolved Hide resolved
@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

Personally, just from a glance, the amount of issue templates seem a bit excessive (ten in total) — so much so that it struggles to fit on my screen. The more templates we have, the more users have to think before choosing a category which adds a considerable barrier to entry.
...
Bug report: (unify all different categories too ­— we can't expect people to know the difference between a NixOS module bug and a package bug all the time, and Darwin should ideally just be a checkbox)

Yes, unfortunately the amount of templates (especially from bug report) is due to a GitHub limitation. I can't add the darwin label through a checkbox, for instance. I need to make a whole new template for that. The ideal solution to this problem is to get GitHub to somehow implement if conditions in YAML to only show users the required information at any given time.

But, consider that it may be a good idea to make users think before submitting. We have thousands of open issues. A non-insignificant amount of users do no research into their issues before submitting, making them difficult to resolve. Perhaps the slightly higher bar of entry isn't such a bad thing if it'll net us well-thought-through issues.

Tracking issue: (remove: contributors should make those manually)

I agree, but the only two templates I've made are the backport one and the extra bug report ones. The tracking issue template was implemented in #316129, so I'll ask my coincidentally-named counterpart, @Sigmanificient, for an approval before deleting it.

Request: package update — note: we should tell people that diverging forks should count as new packages, while a non-diverging one should be a package update (we should define what does mean, too); they should also check any recent merged PR against master as sometimes people make package update requests only to realize that it's in master but not unstable

I'm surprised that's common enough to warrant concern! I'll add a note.

The order in which these are laid out are also quite strange. Backport requests, then bug reports, build failures, requests for docs/modules/packages, tracking issues (which only existing contributors should make, honestly), and update requests. I suspect the reason is that it's laid out in lexicographic order by default, but that's plainly unintuitive. Based on the frequency of these issue types in my experience, IMO the order should be more:

* Bug report 

* Build failure

* Request: _new_ package

* Request: package _update_

* Request: new NixOS module

* Request: backport to stable

* Request: documentation

* Tracking issue

Yes, for some reason GitHub orders the issue forms based on their filenames. I think this is easily solvable by prefixing them with numbers though.

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

Most comments were addressed. I'll update the testing repo shortly.

Edit: updated!

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

@infinisil the Codeowners script had a weird error. It seems like it breaks when a file expanded from a wildcard has a space?

@SigmaSquadron SigmaSquadron force-pushed the push-sxvxkztqqzvk branch 2 times, most recently from 0322fe4 to 2d22c41 Compare December 28, 2024 08:32
Copy link
Member

@donovanglover donovanglover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. My main concern is the increase in information density. Since labels and descriptions are used already, should placeholders be removed in favor of blank fields?

.github/ISSUE_TEMPLATE/01 bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/01 bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/04 build_failure.yml Outdated Show resolved Hide resolved
@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

should placeholders be removed in favor of blank fields?

Point 1: This is only valid for things like password prompts, where the placeholder is the only indicator of what to add in the text fields. We have descriptions as well.

Point 2: We have labels.

Point 3: Isn't applicable. The only way to get an error is to not answer a required question.

Point 4: On GitHub, the placeholder stays visible when the cursor is on the form field.

Point 5: This is the only point that makes me want to remove all of the placeholders. Counterpoint, if users aren't attracted to text, but to blank space, how can we be sure that they will fill the form correctly? (Assuming they won't read the description text)

Point 6: Quite difficult, considering the significant contrast difference.

Point 7: Isn't applicable, unless it's a value instead of a placeholder.

However, even when using labels, placing important hints or instructions within a form field can still cause the 7 issues mentioned above

Unless I did an oops, all of our placeholders are dummy examples of something that could be filled in. They provide no important information other than helping users know what a "clear and concise description" means.

Accessibility points:

Point 1: Invalid on GitHub, since there are high-contrast themes that should remedy this issue.

Point 2: Isn't applicable due to the reasons above.

Point 3: Fair and valid! I have no way to test (Orca has never worked on my systems), but I hope GitHub has competent a11y engineers behind their form design.


That was a nice read! If I was building a website, I'd absolutely take the advice there to heart. However, I don't think it's fully applicable to GitHub issue forms. I do agree w.r.t reducing information density.

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

All placeholders removed. (Except for the reproducible builds issue. That issue cannot be changed.)
Typos fixed. Capitalisation normalised across issue titles and questions.

The testing area was updated.

@Sigmanificient
Copy link
Member

I added a tracking issue template in order to help to create them in a more consistent way 😅
If people think this is unnecessary, and it causes new user to be lost due to the amount of template, I guess you may remove it

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

Personally I'm inclined towards keeping it. The more the merrier, and more consistency and well-thought-out issues is the goal of this PR. It's the last issue in the list, and it makes sense for the contributor-only issues (The blank escape hatch and the tracking template) to be near the bottom. I'll leave it up to @pluiedev.

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

it's 2025 and we still can't have spaces in file names 🫠

@pluiedev
Copy link
Contributor

I still think this is a bit much, in the face of other projects I've seen which usually only have four or five at maximum. (It still fills my screen, on both mobile and desktop!)

image

Tracking issues IMO can be quite different among themselves (for example, clean ups/removing old versions vs treewide migrations) and I still believe we should just create them manually as I don't see the benefit of having a template like that. We sadly don't get to make some templates selectively visible to some users...

As discussed in NixOS#368656.

Approved-by: Yohann Boniface <[email protected]>
Signed-off-by: Fernando Rodrigues <[email protected]>
One for generic package issues, one for NixOS (Module)
issues, and one for Darwin-specific issues.

Signed-off-by: Fernando Rodrigues <[email protected]>
…, backport requests) into YAML forms

The out of date package report was split into two forms: an update request and a backport request.

Signed-off-by: Fernando Rodrigues <[email protected]>
Also sorts the CI list alphabetically and indents it like the rest of the file.

Signed-off-by: Fernando Rodrigues <[email protected]>
@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Dec 28, 2024

Tracking issue has been removed. With that, I believe all issues have been resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: developer experience 6.topic: policy discussion 9.needs: community feedback 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants