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

misc(github): add github PR and issue templates #5163

Merged
merged 5 commits into from
May 21, 2018
Merged

misc(github): add github PR and issue templates #5163

merged 5 commits into from
May 21, 2018

Conversation

underbyte
Copy link
Contributor

Feature request

Summary of the feature request?

Github supports these templates to help standardize contribution into repository. Whenever I'm filing a ticket/PR having these templates help in the writing process. What submitted here are heavily inspired by the webpack community; more possible format can be found here.

What is the motivation or use case for changing this?
To help standardize the format for writing PR/issue tickets.

How is this beneficial to Ligthhouse?
Help make it easier for people to contribute via PR/issue tickets.

@underbyte underbyte changed the title contrib(github): add github PR and issue templates misc(github): add github PR and issue templates May 9, 2018
Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

thanks so much for putting this together @underbyte! I think this will help contributors a lot 👍

**Provide the steps to reproduce if this is a bug**


**Please mention other relevant information such as the browser version, Node.js version, Lighthouse version, and Operating System.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's borrow from the bug report style below:

**Environment Information**:
Affected Channels:
- [ ] CLI
- [ ] Node Module
- [ ] Extension
- [ ] DevTools
Lighthouse version:
Node.js version:
Operating System:

@@ -0,0 +1,21 @@
---
name: Feature request
Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I understand these will only be used if you append ?template=Feature_request.md to the new issue URL?

If so, I'm not sure we have a use case for the specific issue templates just yet. Maybe we should just start with global issue template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does change the URL but you also get this (at least, if I understand it right).

screen shot 2018-05-14 at 8 38 23 pm

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that's awesome!! They should add that to the post 😆

Do we need the global one then if you always get this option?

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW where did that screenshot come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SS came from webpack repo LINK.

Think the global one would be nice to have? Wouldn't hurt to keep it around.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Think the global one would be nice to have? Wouldn't hurt to keep it around.

I'm mainly just thinking about keeping them in sync since they already seem to differ a bit :) it's not a biggie

Copy link
Member

Choose a reason for hiding this comment

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

Ya let's nuke the global issue template. The specific ones are good 'nuff.


<!-- Link any documentation or information that would help understand this change -->

**Other information**
Copy link
Collaborator

Choose a reason for hiding this comment

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

WDYT about changing this into **Related Issues/PRs** we frequently end up linking blocking PRs/closing issues/etc

Other seems like something one can always add on their own :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me!


<!-- If this is a bug report use this portion of the template and delete the feature request portion -->

**Describe the *bug* issue**
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this set of questions! WDYT about adding one about relevant existing issues?

<!-- Have you searched for similar open issues? -->
<!-- If yours is a duplicate, consider commenting with additional info or +1'ing instead. -->
Related issues: <!-- #9000 -->

or something

**What is the expected behavior?**


**Provide the steps to reproduce if this is a bug**
Copy link
Collaborator

Choose a reason for hiding this comment

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

we've already established this is the bug part so let's just say Provide the steps to reproduce

**What is motivation or use case for changing this?**


**Please mention other relevant information such as the browser version, Node.js version, Lighthouse version, and Operating System.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can probably just ask where they want the feature (CLI/DevTools/Extension/Node Module)

<!-- Before creating an issue please make sure you are using the latest version. -->

<!-- If this is a feature request use this portion of the template and delete the bug portion -->
**Describe the *feature* request**
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also have a process for new audits we can reference https://github.com/GoogleChrome/lighthouse/blob/master/docs/new-audits.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

went ahead and added this to the feature request template

@underbyte
Copy link
Contributor Author

@patrickhulce Think I got everything. Let me know if anything else is missing.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

if we sync up the global issue template to match the other ones I think we're good to go from my perspective! thanks very much for the work here!


<!-- Have you searched for similar open issues? -->
<!-- If yours is a duplicate, consider commenting with additional info or +1'ing instead. -->
Related issues: <!-- #9000 -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

oops my proposal seems to be out of style a bit with the rest of yours. let's match it up to what you've got here I like that more :)

**Related 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.

Latest commit, I delete the issue template and added your last two feedback to the bug report.



**Provide the steps to reproduce**

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a placeholder step for the URL since that's almost always the first question we ask

1. Run LH on <affected url>

@underbyte
Copy link
Contributor Author

Hey @patrickhulce, anything left on this PR?

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM!

@patrickhulce patrickhulce merged commit 5d366d5 into GoogleChrome:master May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants