-
Notifications
You must be signed in to change notification settings - Fork 126
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
Update pagespeed insight to v5 #97
Comments
Create a new branch and then do a PR from that branch when it's ready for review. Don't commit directly to master unless it's something minor. |
Many thanks for the quick answer. More clear now. |
First of all a big thanks for thinking about what a revision to support V5 would look like, @JuanMaRuiz. Also an extended thanks to @sindresorhus for taking time out to chime in here :) Always nice to see a friendly face. The proposal you suggested aligns quite well with how I would have gone about trying to present a version of the new pagespeed insights report in a CLI. If we wanted to go further we could potentially also give users the ability to specify which parts of the report they want. For example, I may only want Field data. Or just Lab data. As there's now quite a lot of information available in these reports, I can imagine some users only wanting a subset. That said we could also start with something that just updates to V5 and think about configuration options at a later point. Wdyt? |
Hi! @addyosmani
Totally agree with you. We could update to v5 showing a basic report and after that we could improve given to the user the ability of select the type of report he wants, maybe we could generate a report file like lighthouse does. I'm going to start working in the update. In the mean time, do you think (@addyosmani , @sindresorhus ) that we should publish the new psi version on npm? That really adds value to the current users? The PR updating to v4 was merged but it wasn't generated/publish on npm. |
I would update to Pagespeed v5 first and do a release then. After that, we can improve the output in follow-up releases. I don’t see value in releasing v4 as it’s deprecated and best practices there might be outdated. |
SGTM :) |
Agree with that! Let's do the update. Many thanks both of you! |
Hi! @addyosmani and @sindresorhus I have updated to v5 and added the output data we discussed above. Here is how it looks like vs page speed report on the web
Best regards. Update
I assume that we should update the minimum node version in travis configuration filet to v8, but could you confirm that? I have created a new branch where you can check the PR changes JuanMa. |
Hi @addyosmani if you have any time, could you please take a look to the PR and tell me if my approach makes sense for you? Thanks in advance. |
V4 will be deprecated at the end of the month. |
Thanks for the advice @mathieuforest , we are working to update Regards |
Fixed by #98 |
Hi @addyosmani as we discussed in the PR I have been exploring the possibility to migrate to pagespeed v5 and to add the new lighthouse report to psi tool. As you already know the response from pagessped v2 and pagessped v5 are different but I think it is possible to make the upgrade to the new version with not too much effort and adding the new lighthouse data
I don't know if you have any other initial idea about how the data should be shown but I think that we could show the same approach as Page Speed Insights Page:
Summary section (Current overview section)
(Accessibility score, Best practices, SEO and PWA could be added to the flag options)
Field section
Lab Data (Current statistics section)
Opportunities (Current rule set results)
Let me know what do you know about this idea.
Another question, I don't know what is the way of doing the contributions, should I work on my own fork of this repo? or should I create a new branch to start working in the update?
Thanks in advance.
Best regards.
JuanMa.
The text was updated successfully, but these errors were encountered: