-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Improve tutorial through "Defining the plugin structure" section #4
Improve tutorial through "Defining the plugin structure" section #4
Conversation
5a645a9
to
48951b6
Compare
a9f6259
to
a13eb75
Compare
✅ Deploy Preview for spyder-api-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hi Yeimi! Jut FYI, after talking with Carlos I pushed commits fixing a variety of formatting and typographical issues with the tutorial, so I could continue reviewing (particularly using one sentence per line, so the substantive review comments and suggestions can be made cleanly), and to avoid bothering either of us with tedious minor fixes so we can focus on the substantive issues at hand. My review is still in progress and is rather late, but you can expect it submitted early tomorrow. Thanks! |
968524a
to
e638d21
Compare
Just FYI, I pushed a few additional changes fixing unambiguous textual issues (grammar, punctuation, capitalization, etc.), leaving more non-trivial things like repetition, phrasing, wordiness and content-relevant changes for suggestions in this PR. I also made a commit applying and fixing RST/Sphinx formatting, and one adding ref labels for the sections and topics and linking to them where appropriate. |
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 Yeimi! Just had some remaining comments
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.
Ended up having some more substantive changes than I thought since once I read them carefully, many of the sentences here were confusing or ambiguous.
For some reason, it didn't automatically mark all of the batched suggestions as resolved, even though it did successfully apply them. Never seen that before... |
cef97fc
to
65e744c
Compare
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 now, thanks so much for your great work @yeimiyaz !
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 @yeimiyaz and @CAM-Gerlach for your work on this!
Pull Request
Pull Request Checklist
Description of Changes
Modify the first sections of the tutorial.