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

Chore: Add configuration wrapper markdown for the bug report template #10669

Merged
merged 1 commit into from Sep 1, 2018

Conversation

revolter
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

What changes did you make? (Give an overview)

Copied the wrapper markdown from https://github.com/eslint/eslint/blob/master/.github/ISSUE_TEMPLATE.md.

Is there anything you'd like reviewers to focus on?

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jul 24, 2018
@aladdin-add aladdin-add added enhancement This change enhances an existing feature of ESLint accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon accepted There is consensus among the team that this change meets the criteria for inclusion labels Jul 24, 2018
@aladdin-add
Copy link
Member

the change looks good to me. But I'm not sure if the template is still needed.

@revolter
Copy link
Contributor Author

What do you mean by "needed"? It's currently linked from https://github.com/eslint/eslint/blob/master/.github/PULL_REQUEST_TEMPLATE.md (Bug fix).

@aladdin-add
Copy link
Member

we can change the link to .github/ISSUE_TEMPLATE/BUG_REPORT.md, and then remove the identical template. (if it was not used somewhere else)

Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@revolter
Copy link
Contributor Author

@aladdin-add, Could be, as I think it's identical, but it's not a decision I can take. I could, instead, close this and open another one that does the removal if you guys decide it's the way to go.

@aladdin-add
Copy link
Member

@platinumazure thoughts on #10669 (comment) ?

I'm seeing a rationality to remove the identical template!

@platinumazure
Copy link
Member

I'm okay with removing the duplicate template if it won't change the user experience on a different menu. For example, our New Issue link shows 4(?) different options which link to specific templates- not sure if all of those have to be in the same folder. And then we have the default template, and I don't know if that's used elsewhere.

My suggestion would be to create a test repository with the same template setup as our current one, and then make the same change there and see what happens. (Be sure to check on mobile as well, and on tools like GitHub for Windows if they support the use of these templates.) If we see no issues on the test repository, then we can roll it out there. Sound good?

@not-an-aardvark
Copy link
Member

  • Before GitHub supported multiple issue templates, we had a single issue template with several links to other templates, with the expectation that the user would copy-paste the contents of one of the other templates into the issue. These other templates were in the templates/ folder.
  • Now that GitHub supports multiple issue templates, we moved our templates into .github/ISSUE_TEMPLATE/, and we no longer ask users to copy-paste templates into issues. (We still ask users to do this for PRs.)
  • I don't think GitHub's UI uses anything in the templates/ folder. It only uses things in the project root or in .github/.
  • We could remove the issue template from templates if we update the link in the PR template to point to the templates in .github/ISSUE_TEMPLATE. However, it's worth noting that the files in .github/ISSUE_TEMPLATE have some metadata at the top. I'm not sure if this would degrade the user experience when copy-pasting them.

@platinumazure
Copy link
Member

@aladdin-add @not-an-aardvark I'm not clear what the next steps are here. Can this be merged as is, or are changes needed?

Copy link
Member

@not-an-aardvark not-an-aardvark left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@not-an-aardvark
Copy link
Member

@platinumazure This can be merged as-is. It might be a good idea for us to consider removing this file from templates/ entirely, but as-is this is an improvement over the existing situation.

@platinumazure platinumazure merged commit 32f41bd into eslint:master Sep 1, 2018
@platinumazure
Copy link
Member

Merged. Thanks @revolter for contributing!

@revolter
Copy link
Contributor Author

revolter commented Sep 3, 2018

Thanks for merging!

@revolter revolter deleted the patch-2 branch September 3, 2018 09:52
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Mar 2, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion chore This change is not user-facing enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants