Skip to content

docs: remove extra characters in the command line prompt #5153

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

Closed
wants to merge 2 commits into from

Conversation

hroncok
Copy link
Member

@hroncok hroncok commented Apr 22, 2019

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs (you can delete this text from the final description, this is
just a guideline):

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Target the master branch for bug fixes, documentation updates and trivial changes.

@hroncok hroncok added the type: docs documentation improvement, missing or needing clarification label Apr 22, 2019
@hroncok
Copy link
Member Author

hroncok commented Apr 22, 2019

I've used the GitHub web Interface to create this PR. Please squash when merging.

Note that I think those characters there are redundant, but I'm not sure if the bare no special meaning.

@codecov
Copy link

codecov bot commented Apr 22, 2019

Codecov Report

Merging #5153 into master will increase coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5153      +/-   ##
==========================================
+ Coverage   95.79%   96.06%   +0.27%     
==========================================
  Files         114      114              
  Lines       25825    25825              
  Branches     2550     2550              
==========================================
+ Hits        24740    24810      +70     
+ Misses        757      705      -52     
+ Partials      328      310      -18
Impacted Files Coverage Δ
src/_pytest/fixtures.py 97.93% <0%> (+0.27%) ⬆️
testing/test_assertrewrite.py 84.04% <0%> (+0.29%) ⬆️
testing/test_capture.py 99.24% <0%> (+0.3%) ⬆️
testing/test_config.py 99.49% <0%> (+0.33%) ⬆️
testing/test_pdb.py 99.19% <0%> (+0.4%) ⬆️
src/_pytest/pytester.py 91.1% <0%> (+0.43%) ⬆️
src/_pytest/assertion/rewrite.py 95.47% <0%> (+0.48%) ⬆️
testing/python/fixtures.py 99.08% <0%> (+0.57%) ⬆️
src/_pytest/junitxml.py 95.2% <0%> (+0.63%) ⬆️
testing/test_parseopt.py 97.97% <0%> (+0.8%) ⬆️
... and 9 more

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 34bc594...793b778. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Apr 23, 2019

Please squash when merging.

Not possible (#4361).

I think those examples should be auto-generated, see tox -e regen.

@@ -0,0 +1,2 @@
Remove extra characters in the shown command line prompts in the test discovery
documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

btw: I do not think we need a changelog for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

me neither. creating this was pain via github.

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/pytest-dev/pytest/blob/master/changelog/README.rst

trivial: fixing a small typo or internal change that might be noteworthy.

Note sure if the "might be noteworthy" part applies to fixing typos, but I think this list should be updated to say not to create this just for a trivial typo fix.

Copy link
Member

Choose a reason for hiding this comment

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

Oh excellent point, thanks for pointing that out! We should update that templates to mean that trivial is meant to actual code or dependency changes, but not to small documentation changes in general. 👍

@blueyed blueyed changed the title Remove extra characters in the command line prompt docs: remove extra characters in the command line prompt Apr 23, 2019
@blueyed
Copy link
Contributor

blueyed commented Apr 23, 2019

I think those examples should be auto-generated, see tox -e regen.

Probably means that they are currently not updated then, and that this PR is good to do so in the future.

@@ -206,7 +206,7 @@ You can always peek at the collection tree without running tests like this:

.. code-block:: pytest

. $ pytest --collect-only pythoncollection.py
Copy link
Member

Choose a reason for hiding this comment

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

They are there to not run regendoc, because regendoc uses the line starting as $ to mean to execute the shell command:

https://github.com/pytest-dev/regendoc/blob/27e3652723659d8b2d7a5311567480fccc49cbe2/regendoc/parse.py#L82-L84

Copy link
Member Author

Choose a reason for hiding this comment

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

So this PR is probably bogus. Should I open an issue to have a mechanism for this that doesn't uglify the docs?

Copy link
Member

Choose a reason for hiding this comment

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

Good idea, I would kindly ask to create it in https://github.com/pytest-dev/regendoc though. 👍

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

They are there to not run regendoc

Why though? Shouldn't we have working examples for those also?

Copy link
Member

Choose a reason for hiding this comment

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

You are right, probably they should

@nicoddemus
Copy link
Member

Probably means that they are currently not updated then, and that this PR is good to do so in the future.

Yep, missed your comment before commenting earlier.

Thanks a lot for the quick and thoughtful contribution @hroncok, but I guess we can close this. 👍

@hroncok hroncok closed this Apr 24, 2019
@hroncok hroncok deleted the hroncok-docs-pythoncollection-prompt branch April 24, 2019 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants