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

Use issue templates for creating issues #13222

Merged
merged 15 commits into from Aug 25, 2023

Conversation

Emojigit
Copy link
Contributor

@Emojigit Emojigit commented Feb 18, 2023

This PR uses issue templates to replace the legacy, markdown-only issue form. This forces everyone who want to submit an issue (either bug report or feature request) must follow the formats.

Two notifications are added at the template choose page: one to direct MTG issues to MTG's issue tracker, another to guide issue submitters submitting third-party games issues to search for their issue trackers on CDB.

This PR is Ready for Review. As the main branch of my fork, it can now be tested directly in my fork's create issue page. Click here to pick a template.

I don't know can I create such type of template for PRs or not, but I don't see the need of doing so. We expect PR creators to know how to handle the formats of the markdown description, and should have the freedom of changing it.

required: true
- type: input
attributes:
label: CPU model
Copy link
Member

Choose a reason for hiding this comment

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

Is CPU really necessary for issues? I know it's in the original bug report template but I don't know whether it would affect the reproduction of any issues, as opposed to GPUs that can have very unique graphical issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we should ask the original author @nerzhul.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my own opinion, I think CPU model is not a must to provide, as microcode bugs are usually not what Minetest Developers can handle. However, providing as much information as possible is always recommended for spotting the problem more accurately.

@Desour
Copy link
Member

Desour commented Feb 18, 2023

TBH, I prefer the simple markdown thing we have now.
If someone submits a low quality issue and doesn't even fill out the template, the issue can simply be closed manually.
The reason why I don't like this new template is that it is to rigid:

  • Some issues just don't fit exactly into the classic feature request or bug report category. People who open these issues normally know what they're doing.
  • You can't delete or add subsections in the issue, right? This makes describing the issue harder.
  • The fields in the bug report template are also too rigid. Most issues don't require knowledge about the gpu or cpu model. But they do need other information, for example the relevant settings.

@rubenwardy
Copy link
Member

Some issues just don't fit exactly into the classic feature request or bug report category. People who open these issues normally know what they're doing.

You can still open blank issues here:

image

The fields in the bug report template are also too rigid. Most issues don't require knowledge about the gpu or cpu model. But they do need other information, for example the relevant settings.

Might be worth making the fields higher level and less specific. For example, a single field rather than individual ones for everything

@Emojigit
Copy link
Contributor Author

Might be worth making the fields higher level and less specific. For example, a single field rather than individual ones for everything

May you specifically point out how the templates can be improved?

@rubenwardy rubenwardy added the Concept approved Approved by a core dev: PRs welcomed! label May 27, 2023
Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

The concept looks good. The implementation has some small issues.

.github/ISSUE_TEMPLATE/config.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yaml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yaml Outdated Show resolved Hide resolved
Co-authored-by: Muhammad Rifqi Priyo Susanto <muhammadrifqipriyosusanto@gmail.com>
Copy link
Member

@srifqi srifqi left a comment

Choose a reason for hiding this comment

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

This PR looks good to me.

@Emojigit
Copy link
Contributor Author

I've added "Active renderer" and "Irrlicht device" fields into my issue template, hence merging f41e9e3 into my PR.

@srifqi srifqi merged commit 54eacca into minetest:master Aug 25, 2023
required: false
- type: input
attributes:
label: Irrlicht device
Copy link
Member

Choose a reason for hiding this comment

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

"Irrlicht device" is a pretty meaningless question, no one is going to be able to answer this. And it's something that's pretty much fixed based on the platform. I'd just remove this for now

placeholder: "Example: NVIDA GeForce RTX 4090"
validations:
required: false
- type: input
Copy link
Member

Choose a reason for hiding this comment

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

This is a duplicate of active renderer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the PR is already merged, should I create a new PR to remove these?

grorp pushed a commit to grorp/minetest that referenced this pull request Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants