-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
Update readme #21
Update readme #21
Conversation
README.rst
Outdated
`bugs.python.org <https://bugs.python.org/>`_. | ||
Bug reports are welcome! You can use the `issue tracker | ||
<https://bugs.python.org>`_ to report bugs, and/or submit pull requests `on | ||
Github <https://github.com/python/cpython>`_. |
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.
GitHub with capital H
:)
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!
README.rst
Outdated
@@ -144,14 +137,14 @@ is produced, something is wrong. | |||
By default, tests are prevented from overusing resources like disk space and | |||
memory. To enable these tests, run ``make testall``. | |||
|
|||
IMPORTANT: If the tests fail and you decide to mail a bug report, *don't* | |||
IMPORTANT: If the tests fail and you decide to file a bug report, *don't* | |||
include the output of ``make test``. It is useless. Run the failing test | |||
manually, as follows:: |
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 paragraph is a bit harsh, from the uppercase "IMPORTANT" to calling the output of "make test" "useless".
I would rewrite it as:
If the tests fail, you can run the failing test(s) in verbose mode using:
./python -m test -v test_that_failed
You should then file a bug report and include the output of this command.
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.
Good point, will amend.
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.
Maybe we could keep a consistence, ./python -m test -v test_that_failed
or make test TESTOPTS="-v" test_that_failed
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.
@matrixise I'm not sure I understand your point. I switched to make test TESTOPTS="-v test_that_failed"
because running the tests is suggested with make test
, and there was previously a note about what to do if you built python in a different directory. Reusing make test
with TESTOPTS
means the different directory question just goes away, but we get exactly the same result.
README.rst
Outdated
compiler toolchains ability to optimize across the otherwise arbitrary ``.o`` file | ||
boundary when building final executables or shared libraries for additional | ||
performance gains. | ||
compiler toolchains' ability to optimize across the otherwise arbitrary ``.o`` |
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.
why this character ' ?
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.
It's the possessive form. Maybe a better wording would be "LTO takes advantage of the ability of recent compiler toolchains to optimise..."
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.
yep, because I have not seen the possessive form, but just a mistake in the text.
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 agree, this is significantly less than ideal. Fixing it also showed that there's a typo in "takes advantages".
These include spelling/grammar fixes, removing some outdated prose, updating some superceded prose, and adding/cleaning up some links.
[skip ci]
Codecov Report
@@ Coverage Diff @@
## master #21 +/- ##
==========================================
- Coverage 82.38% 82.37% -0.02%
==========================================
Files 1428 1427 -1
Lines 351138 350948 -190
==========================================
- Hits 289291 289087 -204
- Misses 61847 61861 +14 Continue to review full report at Codecov.
|
Includes GH-2, pythonGH-70, pythonGH-73, and pythonGH-21.
Backport to 3.5 and 2.7? |
21: warn for keys/items/values/range r=ltratt a=nanjekyejoannah I think I was over-thinking the solution with flags at synbol table generation. I found a simple check at the AST that checks correctness of values assigned to wanr for things like "True" and other keywords and used this check to warn for these calls. This warns for `keys/values/items/range`: Accurate when the statement is in an assignment: `x = range(20)` ` y = d.keys()` Co-authored-by: Joannah Nanjekye <jnanjeky@unb.ca>
Also full coverage for all supported Pythons.
This does some modernization and cleanup of the toplevel README. There is still significant room for improvement, but this hits some of the low-hanging fruit.
For reviewers, the first commit is the real change, the second commit is a simple rewrap.