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: update issues template to be simpler #1065

Merged
merged 1 commit into from
Mar 8, 2018
Merged

Conversation

virkt25
Copy link
Contributor

@virkt25 virkt25 commented Mar 1, 2018

A proposal for a new issues template that is simpler and allows users to ask questions / provide feedback via issues.

fixes #979 and #47


It's hard to change this template by just discussing it so I hope this proposal provides ground for discussion based on something concrete. We can iterate over the content as needed till everyone agrees to a new revision (I think we all agree we need a new template).

P.S. Remember we can always try something out and re-iterate when / if needed

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Related API Documentation was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in packages/example-* were updated

[ ] Documentation issue or request
[ ] Support / Question
[ ] Feedback / Comment / Thanks / Other
</code></pre>
Copy link
Member

Choose a reason for hiding this comment

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

For posterity, I am cross posting from #979 (comment):

Please don't use checklists as a replacement for radio buttons. GitHub shows completed + total number of items from checklists in GitHub issue list (see list of pull requests to see yourself). We don't want every user-reported issue to say "1 of 3" in the listing.

Regarding issue type in particular, we used to have a checklist like that in the past and it did not work well. In my experience, users often don't know what is the type of their report. Things that look like a bug to them may be considered as a missing feature by us. Discussions that start like a question can often evolve into a bug report or a feature request.

IIUC, wrapping the check boxes in <pre><code> will prevent GitHub from treating this content as a checklist. Which is good!

I don't like the rendered output of this formatting, but I can live with it until we find a better solution.

screen shot 2018-03-01 at 10 12 24

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 idea to use <pre><code> came from Angular's Issue template. https://github.com/angular/angular/blob/master/.github/ISSUE_TEMPLATE.md

I saw that comment and agree that we should triage the issue and label it accordingly but I think it may be helpful to know what a user thinks it might be.

That said ... This is something I'd be ok to remove as well if that's what makes sense. A pro of that would be a simpler template. Thoughts @strongloop/sq-lb-apex?

If bug: Steps to reproduce
-->

# Link to reproduction sandbox
Copy link
Member

Choose a reason for hiding this comment

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

I think we eventually need to come up with a solution allowing people to provide us with a reproduction sandbox. I am thinking about adding a new monorepo package sandbox. This package should contain an empty app created via lb4. By keeping the sandbox in monorepo, people can easily run their code against the latest master branch (lerna will link dependencies from monorepo instead of installing from npmjs.org).

Anyhow, I think this is out of scope of this pull request.

Copy link
Contributor Author

@virkt25 virkt25 Mar 2, 2018

Choose a reason for hiding this comment

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

Neat idea! IIUC ... they would clone the entire monorepo to provide the sandbox which in turn makes it easier for us to debug and helps ensure it's being checked against the latest version. But at the same time ... idk how users would feel about cloning the entire monorepo.

Please correct me if I understood this wrong. Or rather ... I'll create a new issue for this discussion.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, your understanding is correct - we will ask people to fork our entire monorepo, create a feature branch to demonstrate the problem and then give us a link to that feature branch.

idk how users would feel about cloning the entire monorepo.

What are your concerns? In LoopBack 3.x, we were asking people to fork & clone our sandbox repo. I personally don't see much difference between forking and cloning a GitHub project with a single package vs. a monorepo with multiple packages.

https://strongloop.com/api-connect-faqs/
https://strongloop.com/node-js/subscription-plans/
HELP US HELP YOU BY PROVIDING AS MUCH OF THE FOLLOWING INFORMATION AS POSSIBLE AND
AVOID DUPLICATES BY SEARCHING BEFORE CREATING A NEW ISSUE.
Copy link
Member

Choose a reason for hiding this comment

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

I am proposing to add a paragraph nudging people to posting questions elsewhere. The fact that we accept questions on GitHub does not mean GitHub is the best place where to ask them.

HELP US HELP YOU BY PROVIDING AS MUCH OF THE FOLLOWING INFORMATION AS POSSIBLE AND
AVOID DUPLICATES BY SEARCHING BEFORE CREATING A NEW ISSUE.

If you have a question then please consider using a more suitable venue:
 - https://stackoverflow.com/questions/tagged/loopbackjs
 - https://groups.google.com/forum/#!forum/loopbackjs
 - https://gitter.im/strongloop/loopback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was avoiding it because it adds extra text to read at the start of the template BUT I do think it's necessary and most other projects have something along those lines.

Also include actual results if bug
-->
## Current Behaviour
<!-- Include link to a sandbox application for reproduction is available -->
Copy link
Member

Choose a reason for hiding this comment

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

Include link to a sandbox application for reproduction is available

This does not read as an English sentence to me - I see two verbs "include" and "is available" (Include link (...) is available).

Few proposals for inspiration:

  • Include a link to a sandbox application which we can use to reproduce the problem.
  • Include a link to a minimal application reproducing the problem. We cannot help you if we don't know how to reproduce the problem you are experiencing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to say if available. That said I'll update this to something along the suggestions you have since those are better.

Immediate support:
https://strongloop.com/api-connect-faqs/
https://strongloop.com/node-js/subscription-plans/
HELP US HELP YOU BY PROVIDING AS MUCH OF THE FOLLOWING INFORMATION AS POSSIBLE AND
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that we are trying to emphasize what's being said here, but the all caps seems like we're screaming at them :P. Can we make this statement bold instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't do bold since this is a comment. But I do see your point and have modified it ... to some extent.

@virkt25 virkt25 self-assigned this Mar 1, 2018
@virkt25 virkt25 force-pushed the update-issues-template branch from e4e385e to 4f9f515 Compare March 2, 2018 06:37
@virkt25
Copy link
Contributor Author

virkt25 commented Mar 2, 2018

Below are a few different proposals. I can make separate commits with each one if it'll help.

Looking at the goal of the task ... to simplify the template, I personally prefer Proposal # 2 since it keeps the template short, simple, to the point.

Proposal # 3 would be ideal if GitHub had a way of changing the template based on the type of issue a user was having.

Proposal 1

<!--
HELP US HELP YOU BY
- Doing a quick search to avoid duplicate issues
- Providing as much information as possible (versions, use case scenarios for features, etc.)

If you have a question then please consider using a more suitable venue:
 - https://stackoverflow.com/questions/tagged/loopbackjs
 - https://groups.google.com/forum/#!forum/loopbackjs
 - https://gitter.im/strongloop/loopback
-->

## I'm submitting a ...
<!-- Check one of the following options with a "x" -->
<pre><code>
[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report
[ ] Feature request <!-- Include a problem statement, use case scenario, example, code samples, etc. -->
[ ] Documentation issue or request
[ ] Other (Support / Question / Feedback / Comment / Thanks)
</code></pre>

## Current Behaviour
<!-- Help us help you! Include a link to a sandbox application which we can use to reproduce the problem. -->

## Expected Behaviour

## Additional Information
<!--
If applicable, Copy + Paste the output of these two commands:
  node -e 'console.log(process.platform, process.arch, process.versions.node)'
  npm ls --prod --depth 0 | grep loopback
-->

Proposal 2

<!--
HELP US HELP YOU BY
- Doing a quick search to avoid duplicate issues
- Providing as much information as possible (versions, use case scenarios for features, etc.)
-->

## Description

## Current Behaviour
<!-- Help us help you! Include a link to a sandbox application which we can use to reproduce the problem. -->

## Expected Behaviour

## Additional Information
<!--
If applicable, Copy + Paste the output of these two commands:
  node -e 'console.log(process.platform, process.arch, process.versions.node)'
  npm ls --prod --depth 0 | grep loopback
-->

Proposal 3

<!--
HELP US HELP YOU BY
- Doing a quick search to avoid duplicate issues
- Providing as much information as possible (versions, use case scenarios for features, etc.)
- Fill the appropriate sections of the template (Bug Report / Feature Proposal / Question) and delete the others.

If you have a question then please consider using a more suitable venue:
 - https://stackoverflow.com/questions/tagged/loopbackjs
 - https://groups.google.com/forum/#!forum/loopbackjs
 - https://gitter.im/strongloop/loopback
-->

## Bug Report

### I'm trying to ...

### LoopBack's current behaviour is ... 

### But I expected LoopBack to ... 

### Steps to reproduce the problem
<!-- Help us help you! Include a link to a sandbox application which we can use to reproduce the problem. -->

#### Additional Information
<!--
Copy + Paste the output of these two commands:
  node -e 'console.log(process.platform, process.arch, process.versions.node)'
  npm ls --prod --depth 0 | grep loopback
-->

---

## Feature Proposal

### Problem statement

### Use Case

### Examples
<!-- Example usage / code samples / etc. -->

---

<!--
If you have a question then please consider using a more suitable venue:
 - https://stackoverflow.com/questions/tagged/loopbackjs
 - https://groups.google.com/forum/#!forum/loopbackjs
 - https://gitter.im/strongloop/loopback
-->
## Questions

### I'm trying to ...

### I've tried ... 

### I've searched ... 
<!-- Include terms searched on GitHub, Stackoverflow, etc.)

@bajtos
Copy link
Member

bajtos commented Mar 2, 2018

I would personally combine your Proposal 1 and 2:

<!--
HELP US HELP YOU BY
- Doing a quick search to avoid duplicate issues
- Providing as much information as possible 
  (versions, use case scenarios for features, etc.)

If you have a question then please consider using a more suitable venue:
 - https://stackoverflow.com/questions/tagged/loopbackjs
 - https://groups.google.com/forum/#!forum/loopbackjs
 - https://gitter.im/strongloop/loopback
-->

## Description/Steps to reproduce

<!--
Help us help you! 
Include a link to a sandbox application which we can use to reproduce the problem. 
-->

## Current Behaviour

## Expected Behaviour

## Additional Information
<!--
If applicable, Copy + Paste the output of these two commands:
  node -e 'console.log(process.platform, process.arch, process.versions.node)'
  npm ls --prod --depth 0 | grep loopback
-->

Let's not bikeshed too much, I am fine with any of

I don't like Option 3 because it involves too much editing of the template. That is something I almost never do personally - I rather delete the entire template and write the issue description in free form.

@virkt25 virkt25 force-pushed the update-issues-template branch from 188d022 to a975263 Compare March 2, 2018 16:38
@virkt25
Copy link
Contributor Author

virkt25 commented Mar 2, 2018

Went with your proposal @bajtos with 2 changes to the wording (changed tense in top header) and change the comment which we can use => that we can use (though I'm not 100% sure which makes more sense grammatically).

@raymondfeng
Copy link
Contributor

The amount of text (comments or not) in the template won't make a reporter's life easier as the raw text will be overwhelming in the issue creation box. See an example below:

image

I propose to keep the template as less text as possible. Additional guidance should be referring to a link or diagram. I wish github markdown allows embedding a page.

Copy link
Contributor

@raymondfeng raymondfeng left a comment

Choose a reason for hiding this comment

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

We need to further reduce the amount of text in the template.

@virkt25
Copy link
Contributor Author

virkt25 commented Mar 2, 2018

Screenshot of what the latest commit would like to a user

image

@raymondfeng
Copy link
Contributor

There are two steps involved:

  1. The raw text area to create an issue:
    I propose to move comments to the bottom.
## Description/Steps to reproduce

## Current & Expected Behavior

## Additional Information
<!-- -->
  1. Rendered content after the issue is created:

We can add a link to issue creation guidance and have a checkbox to indicate that the reporter has followed the recommended approach.

[ ] I have read [the guide on opening issues for LoopBack 4](https://github.com/strongloop/loopback-next/blob/master/CONTRIBUTING.md#reporting-issues)

@bajtos
Copy link
Member

bajtos commented Mar 5, 2018

I personally think it's good to have some guidance directly in the issue template. People often don't read external documentation, even if there is a check box. How often do you read "Terms of use" before you click on the checkbox saying you have read them?

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

As far as I am concerned, this template is a great middle ground between a concise template with no comments and a rich template explaining everything in long comments.

One small suggestion - add a link to our docs where people can find more information about bug reporting, links to Gitter chat and Google mailing group, etc. so that they don't have to look this up themselves.

HELP US HELP YOU, PLEASE
- Do a quick search to avoid duplicate issues
- Provide as much information as possible (reproduction sandbox, use case for features, etc.)
- Consider using a more suitable venue for questions such as Stack Overflow, Gitter, etc.
See http://loopback.io/doc/en/contrib/Reporting-issues.html

@raymondfeng
Copy link
Contributor

raymondfeng commented Mar 5, 2018

What about the following? You can preview it at https://github.com/raymondfeng/loopback4-example-di/issues/new.

## Description/steps to reproduce

## Current and expected behavior

<!-- HELP US HELP YOU, PLEASE:
- Do a quick search to avoid duplicate issues
- Provide as much information as possible (reproduction sandbox, use case for features, etc.)
- Consider using a more suitable venue for questions such as Stack Overflow, Gitter, etc.
-->

- [ ] I have followed the guidance at [http://loopback.io/doc/en/contrib/Reporting-issues.html](http://loopback.io/doc/en/contrib/Reporting-issues.html).

@virkt25
Copy link
Contributor Author

virkt25 commented Mar 5, 2018

Oh I like that but I think it's better to separate Expected and Current Behaviour to make it easier for us and the users. I also thought we wanted to stay away from checkboxes because GitHub reads that as a task adding 1 of 1 completed to the issue + people can just check it without reading the guidance. We can advise, not enforce ... nor should we be if the issue is valid / important / well described free-form.

Here's an updated proposal:

## Feature Proposal / Description / Steps to reproduce

## Current Behaviour

## Expected Behaviour

<!--
HELP US HELP YOU, PLEASE
- Do a quick search to avoid duplicate issues
- Provide as much information as possible (reproduction sandbox, use case for features, etc.)
- Consider using a more suitable venue for questions such as Stack Overflow, Gitter, etc.
  - See http://loopback.io/doc/en/contrib/Reporting-issues.html
-->

@dhmlau
Copy link
Member

dhmlau commented Mar 5, 2018

I like the above proposal.
+1 on not having checkbox for following issue reporting guideline.

@virkt25 virkt25 force-pushed the update-issues-template branch from 5d21cb3 to 49d718d Compare March 6, 2018 02:42
@raymondfeng
Copy link
Contributor

Two minor comments:

  1. American English prefers behavior
  2. I would like to move the link to issue creation out of the comment. The link is useless if not clickable.

Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

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

👍 LGTM I love the concise template.

@virkt25
Copy link
Contributor Author

virkt25 commented Mar 7, 2018

American English prefers behavior

Oops. My Canadian is showing 😅

Moved the link out of the comment, to the bottom of the template.

@virkt25 virkt25 force-pushed the update-issues-template branch from fc7e30e to 3b48197 Compare March 7, 2018 03:09
@virkt25 virkt25 force-pushed the update-issues-template branch from 3b48197 to 6135e23 Compare March 7, 2018 19:13
A proposal for a new issues template that is simpler and allows users to ask questions / provide feedback via issues.

fixes #979 and #47
@virkt25 virkt25 force-pushed the update-issues-template branch from 6135e23 to 9cc3c7f Compare March 8, 2018 01:36
@virkt25 virkt25 merged commit a62b656 into master Mar 8, 2018
@virkt25 virkt25 deleted the update-issues-template branch March 8, 2018 02:28
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.

New Issues Template
7 participants