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

Remove useless code samples and add missing ones #357

Merged
merged 3 commits into from
Sep 21, 2023
Merged

Conversation

curquiza
Copy link
Member

I created scripts to manage code samples (internal only)

  1. I found out the following code samples are still in this repo but not used by the documentation anymore, so I removed them:
meilisearch-dart
- 'faceted_search_2' not found in documentation
- 'get_keys_1' not found in documentation
- 'documents_guide_add_movie_1' not found in documentation
- 'getting_started_communicating_with_a_protected_instance' not found in documentation
- 'faceted_search_facets_1' not found in documentation
  1. I found the following code samples were missing:
meilisearch-dart
- 'get_typo_tolerance_1' not found
- 'update_typo_tolerance_1' not found
- 'reset_typo_tolerance_1' not found
- 'typo_tolerance_guide_1' not found
- 'typo_tolerance_guide_2' not found
- 'typo_tolerance_guide_3' not found
- 'typo_tolerance_guide_4' not found
- 'async_guide_canceled_by_1' not found
- 'facet_search_1' not found
- 'facet_search_3' not found

All of them were missing, because the related features are implemented.

@curquiza curquiza added the skip-changelog The PR will not appear in the release changelogs label Sep 20, 2023
@curquiza
Copy link
Member Author

curquiza commented Sep 20, 2023

@ahmednfwela @brunoocasali be careful before fixing this kind of issue: #340 -> the code samples were not implemented, so the full issue was not done 😊

But still a big thank for the PR @ahmednfwela of course 😄

@ahmednfwela
Copy link
Collaborator

yes my bad, sometimes I create 3/4 prs a day and forget to add the code samples 😅

@curquiza
Copy link
Member Author

bors try

meili-bors bot added a commit that referenced this pull request Sep 20, 2023
@meili-bors
Copy link
Contributor

meili-bors bot commented Sep 20, 2023

try

Build succeeded:

@ahmednfwela
Copy link
Collaborator

@ahmednfwela
Copy link
Collaborator

I made a PR that supersedes this #358
@curquiza

@curquiza
Copy link
Member Author

@curquiza I suggest also fixing the link at the top to
https://github.com/meilisearch/documentation/blob/main/.code-samples.meilisearch.yaml

I changed the description. Before a kind of documentation existed at this link. I removed it. Not all the code samples are present in https://github.com/meilisearch/documentation/blob/main/.code-samples.meilisearch.yaml

I made a PR that supersedes this #358 @curquiza

Thank you!
However, can we still merge this current PR? The goal is to provide an up to date documentation to the users as soon as possible
@brunoocasali will review the other PR when he will come back from Holiday 😊

@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (d110365) 82.03% compared to head (750e82d) 82.03%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #357   +/-   ##
=======================================
  Coverage   82.03%   82.03%           
=======================================
  Files          57       57           
  Lines        1353     1353           
=======================================
  Hits         1110     1110           
  Misses        243      243           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ahmednfwela
Copy link
Collaborator

yes sure! we can merge this PR now, but is there a place that contains all the code samples ?

ahmednfwela
ahmednfwela previously approved these changes Sep 21, 2023
Copy link
Collaborator

@ahmednfwela ahmednfwela left a comment

Choose a reason for hiding this comment

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

bors merge

meili-bors bot added a commit that referenced this pull request Sep 21, 2023
357: Remove useless code samples and add missing ones r=ahmednfwela a=curquiza

I created [scripts to manage code samples](https://github.com/meilisearch/integration-automations/pull/164) (internal only)

1. I found out the following code samples are still in this repo but not used by the documentation anymore, so I removed them:

```bash
meilisearch-dart
- 'faceted_search_2' not found in documentation
- 'get_keys_1' not found in documentation
- 'documents_guide_add_movie_1' not found in documentation
- 'getting_started_communicating_with_a_protected_instance' not found in documentation
- 'faceted_search_facets_1' not found in documentation
```

2. I found the following code samples were missing:

```bash
meilisearch-dart
- 'get_typo_tolerance_1' not found
- 'update_typo_tolerance_1' not found
- 'reset_typo_tolerance_1' not found
- 'typo_tolerance_guide_1' not found
- 'typo_tolerance_guide_2' not found
- 'typo_tolerance_guide_3' not found
- 'typo_tolerance_guide_4' not found
- 'async_guide_canceled_by_1' not found
- 'facet_search_1' not found
- 'facet_search_3' not found
```
All of them were missing, because the related features are implemented.

Co-authored-by: curquiza <[email protected]>
@meili-bors
Copy link
Contributor

meili-bors bot commented Sep 21, 2023

Build failed:

  • integration-tests (dart 3.0.0)

@curquiza
Copy link
Member Author

curquiza commented Sep 21, 2023

yes sure! we can merge this PR now, but is there a place that contains all the code samples ?

Nope
most of them are in the file (https://github.com/meilisearch/documentation/blob/main/.code-samples.meilisearch.yaml) but

  • some of them are in the docs but not in the file (because no curl example related)
  • one of them is only used in the landing Meilisearch page: https://www.meilisearch.com/

I created a script that takes into account these exception to check everything is update to date 😊

Copy link
Collaborator

@ahmednfwela ahmednfwela left a comment

Choose a reason for hiding this comment

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

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Sep 21, 2023

Build succeeded:

@curquiza curquiza merged commit a1ed92c into main Sep 21, 2023
6 checks passed
@curquiza curquiza deleted the fix-code-samples branch September 21, 2023 14:24
meili-bors bot added a commit that referenced this pull request Nov 22, 2023
358: Create code samples from code, a.k.a. code excerpts r=ahmednfwela a=ahmednfwela

# Pull Request
## What does this PR do?

This PR completely automates updating the code samples file [.code-samples.meilisearch.yaml](https://github.com/meilisearch/meilisearch-dart/blob/main/.code-samples.meilisearch.yaml)
it creates a new PRIVATE `tool` that does 2 things
1. updates the sample file from source code (run by contributors)
2. check if the sample file is up to date with source code (run in CI)

the private tool checks dart files in `lib/` and `test/` directory for lines that follow this pattern:
```dart
// #docregion key
anything()
// #enddocregion
```
where `key` is a key in `.code-samples.meilisearch.yaml`

[This run](https://github.com/meilisearch/meilisearch-dart/actions/runs/6259938891/job/16996958519?pr=358#step:4:158) shows CI fail in action, due to format mismatch for key `search_parameter_guide_show_ranking_score_1` between code file and yaml file.

This commit f9109a0 shows how to fix the failed run, by running `dart run ./tool/bin/meili.dart update-samples` in the root directory of the repo.


this should simplify adding samples, and avoid issues like #357

--- 

## Update

I have also added functionality to detect missing and useless keys, see this failing run for example: https://github.com/meilisearch/meilisearch-dart/actions/runs/6262599763/job/17005143489?pr=358#step:4:159

## PR checklist
WIP
- [ ] remove unnecessary indentations from code.
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Ahmed Fwela <[email protected]>
Co-authored-by: Ahmed Fwela <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog The PR will not appear in the release changelogs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants