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

Add insights checker #28

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

barryvdh
Copy link
Contributor

This adds https://github.com/dsentker/phpinsights by @dsentker and checks the Google Pagespeed Insights API for mobile and desktop, when the key is set.

It also shows the screenshots from mobile/desktop and the speed/usability scores.

@barryvdh
Copy link
Contributor Author

Example:

screen shot 2017-03-27 at 22 00 09

@hbakhtiyor
Copy link
Contributor

i think better to use from client side directly, not to overload server

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 7, 2017

@hbakhtiyor What do you mean, calling the Insights API? That shouldn't be very intensive..

@phanan What do you think?

@hbakhtiyor
Copy link
Contributor

@barryvdh yeah, calling directly to Insights API

@phanan
Copy link
Owner

phanan commented Apr 7, 2017

Calling the API shouldn't be a problem.

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 7, 2017

No indeed.

But I meant, what do you think of the rest of the PR?

@phanan
Copy link
Owner

phanan commented Apr 7, 2017

Oh, sorry. Actually IMO breaking the insight results into individual items is a bit too much. I'd prefer using the score for the headline and the rest for expanded details.

@phanan
Copy link
Owner

phanan commented Apr 7, 2017

Also, why do we have two Levels::WARNING again?

@phanan
Copy link
Owner

phanan commented Apr 7, 2017

And sorry for not following up the repo very closely these days, I currently have lots of personal stuff to deal with :(

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 7, 2017

Ah right, one should be ERROR indeed.

We could be to just show the once that are failing, not all the green ones.

Or perhaps add a different options to check using kupo rules, but also a check for Insights. The interface is very nice for it imho :)

@barryvdh
Copy link
Contributor Author

barryvdh commented Apr 7, 2017

And no rush, take your time :)

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.

3 participants