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

add QA to docs #1374

Merged
merged 4 commits into from
Apr 17, 2020
Merged

add QA to docs #1374

merged 4 commits into from
Apr 17, 2020

Conversation

Borda
Copy link
Member

@Borda Borda commented Apr 4, 2020

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 🙃

@Borda Borda added the docs Documentation related label Apr 4, 2020
@mergify mergify bot requested a review from a team April 4, 2020 16:00
@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #1374 into master will not change coverage by %.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1374   +/-   ##
======================================
  Coverage      92%     92%           
======================================
  Files          63      63           
  Lines        3317    3317           
======================================
  Hits         3042    3042           
  Misses        275     275           

@awaelchli
Copy link
Member

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.

@Borda
Copy link
Member Author

Borda commented Apr 4, 2020

@awaelchli could you please add it to this PR as a suggestion?

@awaelchli
Copy link
Member

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
```
Copy link
Member

@awaelchli awaelchli Apr 4, 2020

Choose a reason for hiding this comment

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

Suggested change
```
```
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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

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?

Copy link
Member Author

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...

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Borda and others added 2 commits April 5, 2020 00:32
Co-Authored-By: Adrian Wälchli <adrian.waelchli@students.unibe.ch>
@williamFalcon williamFalcon changed the title add QA to docs [WIP] add QA to docs Apr 5, 2020
@Borda Borda marked this pull request as ready for review April 5, 2020 18:35
@Borda
Copy link
Member Author

Borda commented Apr 8, 2020

changelog need to be rebased on new release #1419

@Borda Borda added this to the 0.7.4 milestone Apr 10, 2020
@Borda Borda changed the title [WIP] add QA to docs add QA to docs Apr 10, 2020
@mergify mergify bot requested a review from a team April 13, 2020 09:42
@Borda Borda added the ready PRs ready to be merged label Apr 13, 2020
@awaelchli
Copy link
Member

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.

@Borda
Copy link
Member Author

Borda commented Apr 13, 2020

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...
but the deprecation shall be discussed in the particular PR/issue as it won't happen very often

@williamFalcon williamFalcon merged commit 1ee2837 into master Apr 17, 2020
@Borda Borda deleted the docs/contrib-QA branch April 18, 2020 00:34
tullie pushed a commit to tullie/pytorch-lightning that referenced this pull request Jun 7, 2020
* 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>
@Borda Borda modified the milestones: 0.7.4, v0.7.x Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants