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

report: hidden group backcompat #13310

Merged
merged 12 commits into from
Nov 11, 2021
Merged

report: hidden group backcompat #13310

merged 12 commits into from
Nov 11, 2021

Conversation

adamraine
Copy link
Member

If the user happens to be using a config with no audits in group "hidden", but they're on v9, this shouldn't affect anything.

@adamraine adamraine requested a review from a team as a code owner November 2, 2021 23:53
@adamraine adamraine requested review from connorjclark and removed request for a team November 2, 2021 23:53
@google-cla google-cla bot added the cla: yes label Nov 2, 2021
@adamraine
Copy link
Member Author

but they're on v9, this shouldn't affect anything.

This is a lie. Sufficient changes to the config could break the expectation. I think the safest approach is to switch to a version check, and merge alongside the v9 bump.

@paulirish paulirish added the 9.0 label Nov 8, 2021
@paulirish
Copy link
Member

but they're on v9, this shouldn't affect anything.

This is a lie. Sufficient changes to the config could break the expectation. I think the safest approach is to switch to a version check, and merge alongside the v9 bump.

are you making a change here?

@adamraine
Copy link
Member Author

are you making a change here?

Since the solution is to look for an LHR with version >=9.0.0 I don't think we can merge this until the version bump. I was planning on making this a part of the version bump PR if possible.

@paulirish
Copy link
Member

are you making a change here?

Since the solution is to look for an LHR with version >=9.0.0 I don't think we can merge this until the version bump. I was planning on making this a part of the version bump PR if possible.

gotcha! yeah totally makes sense.

i think you could use bump-versions.js here and just bump to 8.7.0 so this can land. we wouldn't release 8.7.0 but it'd unblock this dealio.

report/renderer/util.js Outdated Show resolved Hide resolved
package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "lighthouse",
"version": "8.6.0",
"version": "8.7.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

pretty weird, wish I had an alternative idea. At least, how about 9.0.0-alpha.0?

Copy link
Member

Choose a reason for hiding this comment

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

this is fine. i considered it but got scared. but sure.

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.

5 participants