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

docs(new-audits): revise issue template #6233

Merged
merged 3 commits into from
Oct 13, 2018
Merged

Conversation

paulirish
Copy link
Member

there was some nonsense in here about the "full config". But I also wanted to prioritize defining what it looks like in the report.

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

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

LGTM

- How would passing/failing be defined?
#### How is this audit different from existing ones?
#### What % of developers/pages will this impact?
_(Estimates OK, data points preferred)_
Copy link
Member

Choose a reason for hiding this comment

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

put <!--comments--> around this instead of italics?

#### What % of developers/pages will this impact?
_(Estimates OK, data points preferred)_
#### How is the new audit making a better web for end users?
_(Data points preferred)_
Copy link
Member

Choose a reason for hiding this comment

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

put <!--comments--> around this instead of italics?

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

should this be an issue template? I guess we get so few audit suggestions that maybe it's fine to still just link to this page

- How would passing/failing be defined?
#### How is this audit different from existing ones?
#### What % of developers/pages will this impact?
_(Estimates OK, data points preferred)_
Copy link
Member

Choose a reason for hiding this comment

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

_? Trying to format or something else?

Copy link
Member

Choose a reason for hiding this comment

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

In the gh issue they would paste this into this would show up as _italics_

Copy link
Member

Choose a reason for hiding this comment

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

oh I see. Yeah that is confusing if you aren't copy/pasting :)

```
#### Provide a basic description of the audit
#### How would the audit appear in the report?
- What detailed info is shown in all cases, or just failure cases?
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 weird way of phrasing this, but I don't have a great alternative that's also short. What's the goal here?

  • what are all the possible states (pass and fail) of the test results
  • what information should be shown in each case to help the user improve
    ?
    (but phrased better :)

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!

Issue template is a good idea 👍

If the details are tabular, what are the columns?
If not obvious, how would passing/failing be defined? -->
#### How is this audit different from existing ones?
#### What % of developers/pages will this impact?
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW I've always thought this one is a weaker version of what we really want to know here which is "What is the global total impact of this audit?" An audit that affects 100% of developers so insignificantly that no one will notice is not as good as an audit that affects half and has a huge impact to each.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the pro/con weighting for it is also different than it once was because now we have notApplicable and can just hide it, so visual noise is less of an issue (though still a concern in the limit). Now it's more maintenance/run time cost as the potential negative.

@paulirish paulirish merged commit 7cb376f into master Oct 13, 2018
@paulirish paulirish deleted the newaudit-doc-tweak branch October 13, 2018 23:10
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.

None yet

4 participants