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.

Sign up for GitHub

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

Jump to bottom

short_name shouldn't be required #2698

Closed
jakearchibald opened this issue Jul 17, 2017 · 3 comments · Fixed by #4860
Closed

short_name shouldn't be required #2698

jakearchibald opened this issue Jul 17, 2017 · 3 comments · Fixed by #4860
Assignees
@paulirish
Labels
bug P1
Milestone
Sprint Doce: Mar ...

Comments

@jakearchibald
Copy link

https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/audits/webapp-install-banner.js#L49

If the name property is short enough, you don't need short_name. The test should reflect this.

This is either a regression of #348, or wasn't fixed.
@jakearchibald
Copy link
Author

Also in https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/audits/manifest-short-name-length.js

@patrickhulce
Copy link
Collaborator

Yeah looks like in the refactor into https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/gather/computed/manifest-values.js the fixed logic stopped being used even though the code is still there, looks like all we need to do is replace hasShortName with shortNameLength?

@paulirish paulirish added P1 and removed P2 labels Feb 12, 2018
@paulirish paulirish self-assigned this Feb 12, 2018
@paulirish paulirish mentioned this issue Mar 9, 2018
Closed
@paulirish
Copy link
Member

related bit: #686

@paulirish paulirish mentioned this issue Mar 23, 2018
Merged
paulirish added a commit that referenced this issue Apr 25, 2018
@paulirish
justify why we require both name and short_name.
58dbe31 
fixes #2698
see #23 (comment)
paulirish added a commit that referenced this issue Apr 26, 2018
@paulirish
justify why we require both name and short_name.
046847f 
fixes #2698
see #23 (comment)
@patrickhulce patrickhulce mentioned this issue May 18, 2018
Closed
@JoelEinbinder JoelEinbinder mentioned this issue Jun 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@paulirish paulirish

Labels
bug P1
Projects
None yet
Milestone
Sprint Doce: Mar 26 - April 6
3 participants
@paulirish @jakearchibald @patrickhulce