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

Fix phpdoc block of IndexesResults constructor #398

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

jonatanrdsantos
Copy link
Contributor

@jonatanrdsantos jonatanrdsantos commented Oct 14, 2022

This class has a bad constructor. It's receiving an array instead of an object as a parameter, which is not a good practice, and PHPdoc does not have a good implementation for documenting nested arrays, so for now what we can do is remove the doc-block, and when a better implementation comes in, we updated doc-block as well.

Related issue

Fixes #397

What does this PR do?

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@brunoocasali
Copy link
Member

Hello @jonatanrdsantos,
Thank you very much for contributing to Meilisearch ❤️.
However, I am not available on the weekend, but I will be back on Monday 😊.

PS: This message was sent automatically!

@jonatanrdsantos jonatanrdsantos force-pushed the feature/fix-error-phpdoc branch 3 times, most recently from 34c8da6 to c9a74e0 Compare October 14, 2022 12:47
@jonatanrdsantos jonatanrdsantos force-pushed the feature/fix-error-phpdoc branch from c9a74e0 to 7d8e091 Compare October 14, 2022 13:05
@brunoocasali
Copy link
Member

Yes indeed, can you create a descriptive issue explaining how the correct behavior should be implemented?

We can work on that in the future then...

Thanks a lot for your time :)

@brunoocasali brunoocasali self-requested a review October 14, 2022 13:13
Copy link
Member

@brunoocasali brunoocasali 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

@bors
Copy link
Contributor

bors bot commented Oct 14, 2022

Build succeeded:

@bors bors bot merged commit c8778dc into meilisearch:main Oct 14, 2022
@brunoocasali
Copy link
Member

This message is sent automatically

Thanks again for contributing to Meilisearch ❤️
If you are participating in Hacktoberfest, and you would like to receive some gift from Meilisearch too, please complete this form.

@brunoocasali brunoocasali added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 17, 2022
@norkunas
Copy link
Collaborator

This is not a fix. It just removes a proper typehint that phpDocumentor does not support. So I think it should be fixed upstream, no?

@jonatanrdsantos
Copy link
Contributor Author

Hi @norkunas yes, I was not able to fix it, so I removed it as it was causing errors.

Meilisearch probably will work on a backlog of issues for documenting the code as the docs get introduced recently.

If you or someone can fix it, I will be glad to learn from it. I'm quite new to docs tools so will be very teachable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix invalid Fqsen in docs generation
3 participants