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 structured search atom for filtering by results that differ #1463

Merged
merged 4 commits into from
Sep 5, 2019

Conversation

lukebjerring
Copy link
Contributor

Fixes #1460

Description

Adds handling of is:different atom, which will return true for rows where the size/cardinality of the set of result statuses is not 1.

Review Information

  • Visit the deployed environment
  • Search for is:different
  • See only rows that have differing values.
  • Try queries with 2, 3, and 4 runs, ensure behaves as expected.

@foolip
Copy link
Member

foolip commented Sep 4, 2019

Staging deployment failed, so I can't test this live.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Looking forward to testing this, LGTM % nits

api/query/README.md Outdated Show resolved Hide resolved



See [#meta-qualities](Meta qualities) above for more information on other
Copy link
Member

Choose a reason for hiding this comment

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

There's just different now, but is flaky one you want to add in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep :)

api/query/cache/index/filter.go Outdated Show resolved Hide resolved
@@ -117,6 +117,15 @@ func (Link) Size() int { return 1 }
// Size of Count is the sum of the sizes of its constituent ConcretQuery instances.
func (c Count) Size() int { return size(c.Args) }

// Size of Is depends on the quality.
Copy link
Member

Choose a reason for hiding this comment

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

What are these Size() methods for, what happens if they return the wrong value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "size" of the query being performed; not sure exactly when it's used. @mdittmer could answer that.

@@ -117,6 +117,15 @@ func (Link) Size() int { return 1 }
// Size of Count is the sum of the sizes of its constituent ConcretQuery instances.
func (c Count) Size() int { return size(c.Args) }

// Size of Is depends on the quality.
func (q MetadataQuality) Size() int {
switch q {
Copy link
Member

Choose a reason for hiding this comment

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

return 1 with a comment and/or assert would be better IMHO than scaffolding for extensions that may never come.

lukebjerring and others added 2 commits September 5, 2019 08:02
Co-Authored-By: Philip Jägenstedt <philip@foolip.org>
@lukebjerring
Copy link
Contributor Author

Staging deployment failed, so I can't test this live.

Are you expecting to see the comment from wpt-pr-bot? We've migrated to GitHub deployments already :)

image

@foolip
Copy link
Member

foolip commented Sep 5, 2019

Yes, I was expected the comments still, fail :)

@lukebjerring lukebjerring merged commit af72dd6 into master Sep 5, 2019
@lukebjerring lukebjerring deleted the different branch September 5, 2019 13:58
foolip added a commit that referenced this pull request May 8, 2024
These noticed have been there since 2019:

#1445
#1463
#1471
#1498

They've been stable since then, so not beta.
past pushed a commit that referenced this pull request May 30, 2024
These noticed have been there since 2019:

#1445
#1463
#1471
#1498

They've been stable since then, so not beta.
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.

Missing ability to filter by *any* difference to spot changes in behavior
3 participants