Skip to content

Update geo javascript playground #587

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 18 commits into from
Nov 11, 2021
Merged

Update geo javascript playground #587

merged 18 commits into from
Nov 11, 2021

Conversation

@CaroFG
Copy link
Contributor

@CaroFG CaroFG commented Nov 2, 2021

Replace 20 European cities dataset with 32419 world cities, add sort by population feature.

@curquiza curquiza requested a review from bidoubiwa November 3, 2021 09:32
@CaroFG CaroFG force-pushed the update-geo-javascript branch from 08f26ac to 0378254 Compare November 9, 2021 11:02
curquiza
curquiza reviewed Nov 9, 2021
View reviewed changes
Copy link
Member

@curquiza curquiza 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.

Should we put dataset in this repo? Maybe playgrounds/geo-javascript/setup/world-cities.json could be in the dataset repo?

It's as you wish since it's a small dataset :)

@CaroFG
Copy link
Contributor Author

CaroFG commented Nov 9, 2021 •
edited
Loading

I already added it to the dataset repo: meilisearch/datasets#9
Tell me if you want me to delete it from this one :)

bidoubiwa
bidoubiwa suggested changes Nov 10, 2021
View reviewed changes
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

The dataset should not stay in the repo, you can remove the whole setup folder that I initially added here for reviewers since there was no remote MeiliSearch host :)

@CaroFG
Copy link
Contributor Author

CaroFG commented Nov 10, 2021

The dataset should not stay in the repo, you can remove the whole setup folder that I initially added here for reviewers since there was no remote MeiliSearch host :)

I should remove the first sentence of the README and modify the contributing guide?

To run this playground you will first have to set up your MeiliSearch accordingly to the playground needs. To do so please follow the steps in our [contributing guide](./CONTRIBUTING.md#-geo-search-playground).

@CaroFG CaroFG force-pushed the update-geo-javascript branch from 7bdaeab to ad1ff93 Compare November 10, 2021 15:52
@CaroFG
Copy link
Contributor Author

CaroFG commented Nov 10, 2021

Since an online dataset is now provided, I updated the information on the readme and the contributing guide

@CaroFG CaroFG requested review from bidoubiwa and curquiza November 10, 2021 16:20
bidoubiwa
bidoubiwa reviewed Nov 10, 2021
View reviewed changes
@CaroFG @bidoubiwa
Update playgrounds/geo-javascript/README.md
b3fe2ee 
Co-authored-by: cvermand <33010418+bidoubiwa@users.noreply.github.com>
bidoubiwa
bidoubiwa approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

🔥 Thanks for this PR 🔥

@bidoubiwa
Copy link
Contributor

bidoubiwa commented Nov 11, 2021

bors merge

@bors
Copy link
Contributor

bors bot commented Nov 11, 2021

Build succeeded:

@bors bors bot merged commit 97af2f3 into meilisearch:main Nov 11, 2021
@bidoubiwa bidoubiwa added the skip-changelog The PR will not appear in the release changelogs label Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@curquiza curquiza Awaiting requested review from curquiza

1 more reviewer

@bidoubiwa bidoubiwa bidoubiwa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip-changelog The PR will not appear in the release changelogs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@CaroFG @bidoubiwa @curquiza