-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
add QA to docs #1374
add QA to docs #1374
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1374 +/- ##
======================================
Coverage 92% 92%
======================================
Files 63 63
Lines 3317 3317
======================================
Hits 3042 3042
Misses 275 275 |
Not really a QA but maybe adding a note that inspecting the docs for formatting errors is also important when updating the docs (not just making the tests pass). If this is just for QA then we can consider adding this later. |
@awaelchli could you please add it to this PR as a suggestion? |
I can't add as suggestion, if you want you can copy paste from here. |
git rebase master | ||
# follow git instructions to resolve conflists | ||
git push -f | ||
``` |
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.
``` | |
``` | |
3. **Should I add type hints in the signature or in the docs, or both?** | |
We prefer to have the type hints in the method/function signature. | |
They should not be added to the docstring because Sphinx will automatically generate them for us. This avoids duplication. |
here is one possible thing we encounter frequently.
It could also go to the documentation section? what do you think?
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.
this doc is in docs - https://pytorch-lightning.readthedocs.io/en/latest/CONTRIBUTING.html
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.
i looked at the link, i don't see a recommendation regarding type hints. am i missing something?
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.
this change is not published yet...
Co-Authored-By: Adrian Wälchli <adrian.waelchli@students.unibe.ch>
changelog need to be rebased on new release #1419 |
One more idea: Maybe a QA for how to deprecate and keep backward compatibility. For how many versions should there be a warning until it gets removed? etc. |
it depends on what change - 0.2 for small or till major version for large... |
* add QA to docs * not about doc updates by @awaelchli * Apply suggestions from code review Co-Authored-By: Adrian Wälchli <adrian.waelchli@students.unibe.ch> * help Co-authored-by: Adrian Wälchli <adrian.waelchli@students.unibe.ch>
What does this PR do?
just WIP collecting the frequent questions... @awaelchli do you have any other suggestions?
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃