-
Notifications
You must be signed in to change notification settings - Fork 183
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
DOC: Add Wachiou BOURAIMA GSoC'24 first Blog post #889
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the post, LGTM. See below for a couple of small changes so the CI passes.
|
||
For my next task, I'll first apply the advice and comments from my first task, adding the **keyword_only** decorator after all the necessary reviews and member approval, on all the functions concerned next, then I'll start and finish, integrating the lazy loading feature. | ||
|
||
🥰 Thank you for reading. Your comments are most welcome, and I learn from them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a new empty line at the end of the file, otherwise the pre-commit hooks complain.
Also, as for the codespell (summary of the CI log, check it yourself just in case):
- In
gltf.py
--> Problem with the word "metrices". - In the
2021-16-08-gsoc-devmessias-11 post
--> Problem with the word "woking". - In
test_stream.py
--> Problem with word "envent".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@itellaetxe, I think code typo fixes should come on a different PR. They are unrelated to the blog. Not sure why CI flagged it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, at least the 2021 blog typo should be addressed, no point on making a new PR just for that typo. For the rest of the code, yes, I agree, maybe that could be done in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @robinroy03 @itellaetxe !
Thank you very much for your comments.
@itellaetxe , yes, in fact I need to make an empty live at the end and remove the white spaces to correct pre-commit errors.
LGTM. |
Squashing commits would be nice for clear history. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @WassCodeur,
Overall, it looks good. See below some small comments to fix
docs/source/posts/2024/2024-05-28-first-post-wachiou-bouraima.rst
Outdated
Show resolved
Hide resolved
docs/source/posts/2024/2024-05-28-first-post-wachiou-bouraima.rst
Outdated
Show resolved
Hide resolved
FIX: formatting and typos in my first blog post RF: Remove empty lines and capitalize DIPY and FURY in first blog post and project details
23e3fa2
to
a467479
Compare
@skoudoro @WassCodeur This one is also working great! |
Thank you @m-agour . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, merging
Changes: