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

core(is-on-https): add missing space in description #11466

Merged
merged 2 commits into from
Sep 21, 2020
Merged

core(is-on-https): add missing space in description #11466

merged 2 commits into from
Sep 21, 2020

Conversation

qwright10
Copy link
Contributor

Summary

This PR adds a missing space to the end of a line in the Uses HTTPS audit description. Currently, the description appears as servedover, but should be served over

Related Issues/PRs

None

@qwright10
Copy link
Contributor Author

Some testing errors need to be fixed. Marking as draft (for now).

@connorjclark
Copy link
Collaborator

should just need to do yarn upgrade:sample-json

@@ -18,7 +18,7 @@ const UIStrings = {
/** Description of a Lighthouse audit that tells the user *why* HTTPS use *for all resources* is important. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
description: 'All sites should be protected with HTTPS, even ones that don\'t handle ' +
'sensitive data. This includes avoiding [mixed content](https://developers.google.com/web/fundamentals/security/prevent-mixed-content/what-is-mixed-content), ' +
'where some resources are loaded over HTTP despite the initial request being served' +
'where some resources are loaded over HTTP despite the initial request being served ' +
Copy link
Collaborator

@connorjclark connorjclark Sep 21, 2020

Choose a reason for hiding this comment

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

ahem this is why I think we should disable max-len lint rule for these few lines and just use one long string.

Copy link
Collaborator

@patrickhulce patrickhulce Sep 21, 2020

Choose a reason for hiding this comment

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

was there a discussion about this before?

We could always write a lint rule to find /\S' \+/ and ban it 😆

Copy link
Member

@brendankenny brendankenny Sep 21, 2020

Choose a reason for hiding this comment

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

our lint settings already allow this string to break max-len because it has a url in it :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. No reasonable way to locate it. GitHub comment search is non-existent.

Copy link
Collaborator

Choose a reason for hiding this comment

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

jk "word wrap" was a pretty good search term. #9105 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

our lint settings already allow this string to break max-len because it has a url in it :)

there's no emoji reaction awesome enough to react to this so here you go

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

jk "word wrap" was a pretty good search term. #9105 (comment)

heh, oh apparently I agreed with you then too.

@connorjclark connorjclark changed the title misc: add missing space in description core(is-on-https): add missing space in description Sep 21, 2020
@connorjclark
Copy link
Collaborator

Thanks @qwright10!

@qwright10
Copy link
Contributor Author

Marking as "not draft." Seems yarn update:sample-json worked perfectly. Thanks @connorjclark

@devtools-bot devtools-bot merged commit 385ab50 into GoogleChrome:master Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants