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

Revise GitHub templates #12574

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

FranciscoPombal
Copy link
Member

@FranciscoPombal FranciscoPombal commented Apr 21, 2020

Add a checklist and guidance comments

Separated from #11896 for easier review.

Previews:

  • Selection screen after clicking "New issue"

    Click to show

    selection

  • Bug report

    Click to show

    bug

  • Feature request

    Click to show

    feature

@xavier2k6
Copy link
Member

Does this or the other PR #11896 close #8330?

@FranciscoPombal
Copy link
Member Author

@xavier2k6 technically, that issue should have been closed with the merging of the linked PR in the last comment (fixed that now). These new PRs are just follow-ups that improve on that original ones, that arose from my additional experience in managing the issue tracker.

.github/ISSUE_TEMPLATE.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE.md Outdated Show resolved Hide resolved
@LordNyriox
Copy link
Contributor

@FranciscoPombal: Not strictly related to the PR itself, but I noticed that both the old template and this one do not seem to have a defined layout for "feature requests".

For instance, submitting a log entry for a behavior that does not yet exist, does not seem viable to me.

Any advice on this?

@FranciscoPombal
Copy link
Member Author

FranciscoPombal commented Apr 23, 2020

@FranciscoPombal: Not strictly related to the PR itself, but I noticed that both the old template and this one do not seem to have a defined layout for "feature requests".

For instance, submitting a log entry for a behavior that does not yet exist, does not seem viable to me.

Any advice on this?

Ok, I have changed things around a bit. Now there are separate templates for bug reports and feature requests, as well as links to the relevant support communities. Additionally, I threw in a simple PR template. When a user clicks "New issue", now they will get something similar to this this: https://github.com/microsoft/TypeScript/issues/new/choose

As a bonus, GitHub will no longer complain about "You are using an old version of issue templates. Please update to the new issue template workflow. Learn more" (check this here: https://github.com/qbittorrent/qBittorrent/blob/master/.github/ISSUE_TEMPLATE.md)

@FranciscoPombal FranciscoPombal changed the title Revise ISSUE_TEMPLATE.md Revise GitHub templates Apr 23, 2020
@FranciscoPombal
Copy link
Member Author

FranciscoPombal commented Apr 23, 2020

I have pushed the changes to my master branch. The bug report and feature request templates are not showing up, but I think that is because my repo is a fork: EDIT: I fixed it.

Try it out here: https://github.com/FranciscoPombal/qBittorrent/issues/new/choose

Here is what it looks like for a user:
image

Note that the headers in the files like the following are metadata for github only and do not show up when the user is creating the issue:

---
name: Feature Request
about: Suggest a new feature or enhancement for qBittorrent.
title: ''
labels: 'Feature request'
assignees: ''
---

As a bonus, feature requests get automatically labeled on creation.

@xavier2k6
Copy link
Member

xavier2k6 commented Nov 8, 2020

<!--
###############################################################################
WARNING!
IGNORING THE FOLLOWING TEMPLATE WILL RESULT IN THE ISSUE BEING CLOSED
AS INCOMPLETE/INVALID
DO NOT DELETE HEADINGS/SECTIONS (LEAVE BLANK IF NOT APPLICABLE)
###############################################################################
Please make sure the description is worded/structured well enough to be understood.
Provide steps to reproduce the issue.
Provide as much context/additional relevant information/examples as possible.
Suggested solution (if applicable).
Do not forget about the mandatory attachments!.

For more information consult the Contributing Guidelines at https://github.com/qbittorrent/qBittorrent/blob/master/CONTRIBUTING.md.
################################## IMPORTANT ###################################
Type your text under the HEADINGS/SECTIONS below (LEAVE BLANK IF NOT APPLICABLE)
Use the Preview tab before posting to ensure correct formatting.
###############################################################################
-->
#### qBittorrent version/architecture (32bit/64bit)

#### Operating system/version

#### (If on windows), please include (FULL version displayed in `command prompt` window) e.g. `[Version 10.0.19042.xyz]`

#### (If on Linux), please include `libtorrent-rasterbar` and `Qt` version

#### Install method (Official installer/Self-compiled/Other - please specify)

#### What is the problem

#### What is the expected behavior

#### Detailed steps to reproduce the problem

#### Extra info (if any)

#### Mandatory Attachments (log(s) and settings file)

How about something like this??
Big wall of text to go through currently in this PR (that I can see people - straight away, deleting & writing their own thing.)
I know, I certainly don't like it!!

Result:

qBittorrent version/architecture (32bit/64bit)

Operating system/version

(If on windows), please include (FULL version displayed in command prompt window) e.g. [Version 10.0.19042.xyz]

(If on Linux), please include libtorrent-rasterbar and Qt version

Install method (Official installer/Self-compiled/Other - please specify)

What is the problem

What is the expected behavior

Detailed steps to reproduce the problem

Extra info (if any)

Mandatory Attachments (log(s) and settings file)

@xavier2k6
Copy link
Member

xavier2k6 commented Nov 8, 2020

The reason I included the FULL Version string is that:
It gives a more searchable text string for determining what OS the qBittorrent user base runs,
This will also aid in the raising "Minimum Supported"/"Dropping" of EOL OS's/because of dependencies/compilers etc.

For example:

Windows 11


Windows 10


Windows 7 / 8 / 8.1


Windows Operating Systems no longer supported as of 4.2.0 Release (December 3rd 2019)


Windows XP / Vista

@xavier2k6
Copy link
Member

xavier2k6 commented Nov 8, 2020

#### qBittorrent version/architecture (32bit/64bit)
<!--Type your text after the `>` to the right-->
#### Operating system/version
<!--Type your text after the `>` to the right-->
#### (If on windows), please include (FULL version displayed in `command prompt` window) e.g. `[Version 10.0.19042.xyz]`
<!--Type your text after the `>` to the right-->
#### (If on Linux), please include `libtorrent-rasterbar` and `Qt` version
<!--Type your text after the `>` to the right-->
#### Install method (Official installer/Self-compiled/Other - please specify)
<!--Type your text after the `>` to the right-->
#### What is the problem
<!--Type your text after the `>` to the right-->
#### What is the expected behavior
<!--Type your text after the `>` to the right-->
#### Detailed steps to reproduce the problem
<!--Type your text after the `>` to the right-->
#### Extra info (if any)
<!--Type your text after the `>` to the right-->
#### Mandatory Attachments (log(s) and settings file)
<!--Paste links/Drop the files on next line below this statement.-->

Or maybe this?

RESULT:

qBittorrent version/architecture (32bit/64bit)

Operating system/version

(If on windows), please include (FULL version displayed in command prompt window) e.g. [Version 10.0.19042.xyz]

(If on Linux), please include libtorrent-rasterbar and Qt version

Install method (Official installer/Self-compiled/Other - please specify)

What is the problem

What is the expected behavior

Detailed steps to reproduce the problem

Extra info (if any)

Mandatory Attachments (log(s) and settings file)

@FranciscoPombal
Copy link
Member Author

Big wall of text to go through currently in this PR (that I can see people - straight away, deleting & writing their own thing.)

Then their issue will be immediately closed and locked as invalid. This is a feature, not a bug, to filter out users who are not willing to put in the minimum effort.

@xavier2k6
Copy link
Member

Then their issue will be immediately closed and locked as invalid. This is a feature, not a bug, to filter out users who are not willing to put in the minimum effort.

You have to take in to account that for some users - looking at that wall of text is basically double dutch & may know nothing about markdown etc so may not know how it's supposed to look etc.

There have been users here, who have made issues & just pasted the SIGABRT crash reports for example with remarks like.....I dunno, just happened....it told me to report it here, so I did. (deleted - the template etc)

OpenSSL/Libtorrent/Boost don't even have that kind of text to read through/fiddle with etc.

Keep it to bare minimum/essentials & it's followed up.

@xavier2k6
Copy link
Member

This is OpenSSL's

<!--
Thank you for your bug report. If this is your first one,
please take the time to read the following lines before posting it.

NOTE:

    If you're asking about how to use OpenSSL, this isn't the right 
    forum.  Please see our User Support resources:
    https://github.com/openssl/openssl/blob/master/.github/SUPPORT.md

Please remember to tell us in what OpenSSL version you found the issue.

For build issues:

    If this is a build issue, please include the configuration output
    as well as a log of all errors.  Don't forget to include the exact
    commands you typed.

    With OpenSSL before 1.1.1, the configuration output comes from the
    configuration command.  With OpenSSL 1.1.1 and on, it's the output
    of `perl configdata.pm --dump`

For other issues:

    If it isn't a build issue, example code or commands to reproduce
    the issue is highly appreciated.
    Also, please remember to tell us if you worked with your own
    OpenSSL build or if it is system provided.

Please remember to put ``` lines before and after any commands plus
output and code, like this:

    ```
    $ echo output output output
    output output output
    ```

    ```
    #include <stdio.h>
    
    int main() {
        int foo = 1;
        printf("%d\n", foo);
    }
    ```
-->

@FranciscoPombal
Copy link
Member Author

@xavier2k6

Suggest -> issue reporting section

Suggest -> feature request section

👍, done.

Suggest -> Changing All headers to something like below to avoid seeing (type here)

### qBittorrent info and operating system(s) <!--Type relevant information below this header-->

### If on Linux, libtorrent-rasterbarandQt versions <!--Type relevant information below this header-->

I understand but I disagree (I have also thought of this in the past). I'm always concerned users will write inside the <!-- -->, which is worse than leaving a (type here).

We already explicitly warn the user to delete each (type here) as they fill out the template in a comment above.
I think it's better to go with (type here) for now, if we still see that many users are not deleting the (type here)s, we can then try alternative solutions such as the one you propose.

@Chocobo1
Copy link
Member

Chocobo1 commented Jun 3, 2021

@Chocobo1, do you have any blocking objections?

no.

@xavier2k6
Copy link
Member

I understand but I disagree (I have also thought of this in the past). I'm always concerned users will write inside the <!-- -->, which is worse than leaving a (type here).

Well, if they do that & we are attempting to tighten up the issue tracker procedures in this PR then we can mark it as invalid can't we? (as they didn't follow our instructions correctly)

One overall checkbox may be sufficient enough for compliance with the instructions set out in markdown text but I won't insist on this.

My opinion from #12574 (comment) would still be the best option & as I said previously this is something that should be looked at in the future. (Maybe 6 months from now?)

I don't think I have any other issues with this PR.

@FranciscoPombal
Copy link
Member Author

FranciscoPombal commented Jun 3, 2021

@xavier2k6

Well, if they do that & we are attempting to tighten up the issue tracker procedures in this PR then we can mark it as invalid can't we? (as they didn't follow our instructions correctly)

Well we should be strict, but not super strict in closing issues for being invalid. A user may make an honest mistake; with the <!-- --> approach, the issue may not be understandable unless we specifically inspect the raw text. With the (type here) approach, the most that can happen in that case is that there is an extra (type here) appended/prepended to whatever the user writes, which is not ideal, but not that harmless, as the content is still all readable - and not a reason alone to close the report.

One overall checkbox may be sufficient enough for compliance with the instructions set out in markdown text but I won't insist on this.

We can always change this in the future if users complain about it. Fortunately I doubt this will be the case. As I've demonstrated before, multiple other projects have been using the "multiple checkbox" method successfully for a long time. They keep the user engaged and alert as they are reading and fulfilling each requirement.

My opinion from #12574 (comment) would still be the best option & as I said previously this is something that should be looked at in the future. (Maybe 6 months from now?)

First we should see how this one does "in production". This might not be the perfect solution until the end of time, but it sure has a much better chance of being more effective than what we have now, and it is proven in other even bigger projects.

Then sure, something based on google forms can be looked at in the future. However, I have expressed my reservations about it, and I'd like to add that some users might not like the fact that they have to interact with an external Google resource just to submit an issue...

I don't think I have any other issues with this PR.

👍

@FranciscoPombal
Copy link
Member Author

@glassez @Chocobo1

@Chocobo1, do you have any blocking objections?

no.

Cool, can we finally get this approved and merged then? I believe @xavier2k6 and @thalieht 's concerns have been addressed (but let me know if there is still some last minute change that you need!).

xavier2k6
xavier2k6 previously approved these changes Jun 3, 2021
Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

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

Cosmetic comment: file names inside ISSUE_TEMPLATE seem to be inconsistent to me (namely using of _template suffix).
Also why some *.md files have UPPERCASE names but others not? Is there any general naming scheme/convention?

@FranciscoPombal
Copy link
Member Author

@glassez

Cosmetic comment: file names inside ISSUE_TEMPLATE seem to be inconsistent to me (namely using of _template suffix).
Also why some *.md files have UPPERCASE names but others not? Is there any general naming scheme/convention?

I followed the conventions/guidelines in GitHub's documentation.

Basically,

  • .github/ISSUE_TEMPLATE directory name must be upper case
  • .github/PULL_REQUEST_TEMPLATE directory name must be upper case
  • the name of the files inside .github/{ISSUE, PULL_REQUEST}_TEMPLATE are case-insensitive. So I chose lower case, to follow the convention of the rest of the files in the repository that don't have a case-sensitivity requirement.
  • SUPPORT.md must (should?) be named exactly like that
  • FUNDING.yml must (should?) be named exactly like that

References:


The .github/PULL_REQUEST_TEMPLATE/pull_request_template.md file name should not contain the _template suffix for consistency with the rest, you are right about that, will push shortly to fix it.

- Separate templates into bug report and feature request
- Add a checklist and guidance comments
- Add a PR template
- Add SUPPORT.md
@FranciscoPombal
Copy link
Member Author

@glassez @Chocobo1 macOS CI failure is obviously spurious.

@glassez glassez merged commit 9f03598 into qbittorrent:master Jun 8, 2021
@glassez
Copy link
Member

glassez commented Jun 8, 2021

Thank you!

@FranciscoPombal FranciscoPombal deleted the contrib-it branch June 8, 2021 18:01
@ArcticGems
Copy link

Maybe when it's time to release 4.4.0 beta, you guys can add another part called something like "[4.4.0 Beta] Bug report" with its own checklist and relevant questions?

@FranciscoPombal
Copy link
Member Author

Maybe when it's time to release 4.4.0 beta, you guys can add another part called something like "[4.4.0 Beta] Bug report" with its own checklist and relevant questions?

These templates can be used without changes for such purposes. It is sufficient for the user to write "4.4.0 Beta" in the version field. If we really do need a clearer separation between issues from that version and others, we'll just label them differently as they're posted.

-->

- qBittorrent version: (type here)
- Operating system(s) where the issue occurs: (type here)
Copy link
Contributor

Choose a reason for hiding this comment

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

After reading a few issues, these "qBittorrent version:" and "Operating system(s) where the issue occurs:" are disrupting the flow of reading. I can't easily filter them to get to the juicy part and furthermore i think they are unnecessary. A simple "(type here)" is sufficient IMO.
Same goes for the section below.

@ArcticGems
Copy link

I think the 'bug report form' that you can find in the 'uBlock-issues' repository is interesting. No more template evading/removing or checkbox mistakes!

It's worth looking into.

PR: uBlockOrigin/uBlock-issues#1651

Check out how it looks here (start a issue, but don't submit) =
https://github.com/uBlockOrigin/uBlock-issues/issues

@xavier2k6
Copy link
Member

xavier2k6 commented Aug 14, 2021

@ArcticGems Thank you so much!!!

@glassez this is a very useable option like what I had similarly suggested in #12574 (comment)

https://github.com/uBlockOrigin/uBlock-issues/blob/master/.github/ISSUE_TEMPLATE/bug_report.yml

https://github.com/uBlockOrigin/uBlock-issues/blob/master/.github/ISSUE_TEMPLATE/config.yml

I'll try to work on this/submit PR over the wknd.

@xavier2k6
Copy link
Member

Here's an example:
new issue template form

@ArcticGems

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants