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

Docs cleanup for contribution #561

Merged
merged 5 commits into from
Mar 21, 2021
Merged

Conversation

Aaron-Robertson
Copy link
Contributor

@Aaron-Robertson Aaron-Robertson commented Mar 19, 2021

Context:

Cleanup documentation for ease of contribution based on this slack conversation.

Description of the Change:

  • Fix outdated contributing link
  • Development guide test section includes pytest-randomly and pytest-mock
  • Contribution challenges, PR template, and issue template links are not dead

Benefits:

Makes it easier for new contributors.

Possible Drawbacks:

N/A

Related GitHub Issues:

N/A

@Aaron-Robertson Aaron-Robertson marked this pull request as draft March 19, 2021 22:15
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #561 (0acc779) into master (de44467) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #561   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files          76       76           
  Lines        8325     8325           
=======================================
  Hits         8189     8189           
  Misses        136      136           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de44467...0acc779. Read the comment docs.

@Aaron-Robertson
Copy link
Contributor Author

Aaron-Robertson commented Mar 19, 2021

Alright @co9olguy @josh146, this one is ready for a bit of feedback, and I have a couple questions/thoughts.

First, for pytest-randomly and pytest-mock I included links to each of their githubs, since pypi lists it as the homepage for both. Preference for github or pypi links?

Next up, could the changelog be more visible?

I struggled to find it outside repo, and thought this line in the readme

See our contributions page for more details, and then check out some of the Strawberry Fields challenges for some inspiration.

could include a link to it as

See our contributions page and changelog for more details, and then check out some of the Strawberry Fields challenges for some inspiration.

Another couple options would be to add it to the website documentation under the contribution and/or development guide PR page(s).

Happy to broaden this cleanup (outside contribution docs) on request!

@Aaron-Robertson Aaron-Robertson changed the title Docs cleanup Docs cleanup for contribution Mar 19, 2021
@Aaron-Robertson Aaron-Robertson marked this pull request as ready for review March 19, 2021 23:23
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution @Aaron-Robertson! Especially a docs cleanup 💯

First, for pytest-randomly and pytest-mock I included links to each of their githubs, since pypi lists it as the homepage for both. Preference for github or pypi links?

GitHub is preferred over PyPI 🙂

Next up, could the changelog be more visible? I struggled to find it outside repo, and thought this line in the readme could include a link to it.

The changelog is actually already available on the website, however it is labelled 'release notes': https://strawberryfields.readthedocs.io/en/stable/development/release_notes.html

But it would definitely be worthwhile to have a link in the readme!

Comment on lines -24 to -30
Available channels:

* `#strawberryfields`: For general discussion regarding Strawberry Fields
* `#sf_projects`: For discussion of research ideas and projects built on Strawberry Fields
* `#sf_bugs`: For discussion of any bugs and issues you run into using Strawberry Fields
* `#sf_interactive`: For discussion relating to the [Strawberry Fields Interactive](https://strawberryfields.ai) web application
* `#sf_docs`: For discussion of the Strawberry Fields [documentation](https://strawberryfields.readthedocs.io)
Copy link
Member

Choose a reason for hiding this comment

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

Good catch removing this!

Comment on lines +4 to +7
The Strawberry Fields test suite requires `pytest <https://docs.pytest.org/en/latest/>`_, `pytest-cov <https://pytest-cov.readthedocs.io/en/latest/>`_, `pytest-randomly <https://github.com/pytest-dev/pytest-randomly>`_, and `pytest-mocks <https://github.com/pytest-dev/pytest-mock/>`_ for coverage reports. These can be installed via ``pip``:
::

$ pip install pytest pytest-cov
$ pip install pytest pytest-cov pytest-randomly pytest-mock
Copy link
Member

Choose a reason for hiding this comment

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

Excellent!

@Aaron-Robertson
Copy link
Contributor Author

Aaron-Robertson commented Mar 20, 2021

Thanks for the review @josh146! I added the changelog to the readme, and thought of one last question. Any desire to run black locally or just fix on GitHub check?

Could make a small development guide section to install black (or even a dev_requirements.txt and include pytest) and a make format in the scripts.

@josh146
Copy link
Member

josh146 commented Mar 21, 2021

Perfect, thanks @Aaron-Robertson!

Could make a small development guide section to install black (or even a dev_requirements.txt and include pytest) and a make format in the scripts.

If you were interesting in tackling this one as well, that would be great :) I'll merge this in now, and best to make a new PR (especially if it is for adding new content).

@josh146 josh146 merged commit 7a66d31 into XanaduAI:master Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants