Skip to content

Add insights checker - #28

Open
barryvdh wants to merge 8 commits into
phanan:masterfrom
barryvdh:feat-insights
Open

Add insights checker#28
barryvdh wants to merge 8 commits into
phanan:masterfrom
barryvdh:feat-insights

Conversation

@barryvdh

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Example:

screen shot 2017-03-27 at 22 00 09

@hbakhtiyor

Copy link
Copy Markdown
Contributor

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

@barryvdh

barryvdh commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

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

@phanan What do you think?

@hbakhtiyor

Copy link
Copy Markdown
Contributor

@barryvdh yeah, calling directly to Insights API

@phanan

phanan commented Apr 7, 2017

Copy link
Copy Markdown
Owner

Calling the API shouldn't be a problem.

@barryvdh

barryvdh commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

No indeed.

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

@phanan

phanan commented Apr 7, 2017

Copy link
Copy Markdown
Owner

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

phanan commented Apr 7, 2017

Copy link
Copy Markdown
Owner

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

@phanan

phanan commented Apr 7, 2017

Copy link
Copy Markdown
Owner

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

@barryvdh

barryvdh commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

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

barryvdh commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

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