Skip to content

v0.28: Faceting settings #1761

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jul 7, 2022
Merged

v0.28: Faceting settings #1761

merged 14 commits into from
Jul 7, 2022

Conversation

guimachiavelli
Copy link
Member

Closes #1704

@guimachiavelli guimachiavelli added this to the v0.28 milestone Jun 28, 2022
@guimachiavelli guimachiavelli linked an issue Jun 28, 2022 that may be closed by this pull request
v0.28: New faceting index setting #1704
Closed
@guimachiavelli guimachiavelli marked this pull request as ready for review June 28, 2022 10:45
maryamsulemani97
maryamsulemani97 suggested changes Jun 28, 2022
View reviewed changes
@guimachiavelli
Copy link
Member Author

Not sure why github actions is complaining. The problematic link is legit. It should be safe to ignore the error.

maryamsulemani97
maryamsulemani97 reviewed Jun 29, 2022
View reviewed changes
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We don't mention that the faceting settings are configurable in learn/advanced/filtering_and_faceted_search.md or anywhere. Maybe we could add something under "Using facets" or work on this after v0.28?

@maryamsulemani97
Copy link
Contributor

Not sure why github actions is complaining. The problematic link is legit. It should be safe to ignore the error.

That's because /reference/api/tasks.md#get-task was replaced with /reference/api/tasks.md#get-one-task in #1746

@guimachiavelli guimachiavelli mentioned this pull request Jun 30, 2022
Closed
17 tasks
@guimachiavelli
Copy link
Member Author

guimachiavelli commented Jun 30, 2022 •
edited
Loading

But that's on the base v0.28 branch, not on this one, right? Or do our actions check against the base PR branch? Also, no error locally.

We don't mention that the faceting settings are configurable in learn/advanced/filtering_and_faceted_search.md or anywhere. Maybe we could add something under "Using facets"

Great idea, will do that.

maryamsulemani97
maryamsulemani97 approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

Looks good, just one minor change 🧛‍♀️

@maryamsulemani97 maryamsulemani97 mentioned this pull request Jul 4, 2022
Closed
Kerollmops
Kerollmops requested changes Jul 4, 2022
View reviewed changes
@maryamsulemani97 @guimachiavelli
v0.28: Add faceting to core concepts+index settings (#1767)
be6cd5c 
* add faceting to index settings+core concepts

* Update learn/configuration/settings.md

* Update settings.md

* Update learn/core_concepts/indexes.md

* Update learn/core_concepts/indexes.md

Co-authored-by: gui machiavelli <[email protected]>

Co-authored-by: gui machiavelli <[email protected]>
gmourier
gmourier requested changes Jul 6, 2022
View reviewed changes
Copy link
Member

@gmourier gmourier left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@guimachiavelli If you decide to explain how facets values are sorted, I made a little suggestion to make it clearer.

Other than that everything seems clear to me ✨

@guimachiavelli @gmourier
Update reference/api/faceting.md
8fa84da 
Co-authored-by: Guillaume Mourier <[email protected]>
gmourier
gmourier approved these changes Jul 6, 2022
View reviewed changes
Copy link
Member

@gmourier gmourier left a comment

Choose a reason for hiding this comment

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

LGTM!

@guimachiavelli guimachiavelli requested a review from Kerollmops July 6, 2022 11:49
Kerollmops
Kerollmops approved these changes Jul 6, 2022
View reviewed changes
Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

LGTM thank you very much!

This was referenced Jul 6, 2022
Merged
Merged
@guimachiavelli
Copy link
Member Author

bors merge

bors bot added a commit that referenced this pull request Jul 7, 2022
@bors @guimachiavelli @maryamsulemani97
Merge #1761
5dd44ec 
1761: v0.28: Faceting settings r=guimachiavelli a=guimachiavelli

Closes #1704 

Co-authored-by: gui machiavelli <[email protected]>
Co-authored-by: gui machiavelli <[email protected]>
Co-authored-by: Maryam <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jul 7, 2022

Build failed:

@guimachiavelli guimachiavelli merged commit 2eda982 into v0.28 Jul 7, 2022
@guimachiavelli guimachiavelli deleted the v0.28-faceting-settings branch July 7, 2022 10:23
@bidoubiwa bidoubiwa mentioned this pull request Aug 25, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kerollmops Kerollmops Kerollmops approved these changes

@gmourier gmourier gmourier approved these changes

@maryamsulemani97 maryamsulemani97 maryamsulemani97 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.28
Development

Successfully merging this pull request may close these issues.

v0.28: New faceting index setting
4 participants
@guimachiavelli @maryamsulemani97 @Kerollmops @gmourier