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

Clarify rx parameter of compileall functions #25857

Merged
merged 2 commits into from May 4, 2021

Conversation

@Jean-Abou-Samra
Copy link
Contributor

@Jean-Abou-Samra Jean-Abou-Samra commented May 3, 2021

This includes nesting re.Pattern (and, while at it, re.Match) in
a class directive so it can be cross-referenced.

This includes nesting re.Pattern (and, while at it, re.Match) in
a class directive so it can be cross-referenced.
@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented May 3, 2021

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@Jean-Abou-Samra

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Looks good, thanks for your contribution! I didn't build the docs manually though to see how it looks.

@JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented May 4, 2021

@Jean-Abou-Samra don't forgot to sign the CLA.

The diff looks ugly, but it is correct, yet I'm not fully happy with the output, (and not fully happy with this kind of diff):

Match Objects

Match objects always have a boolean value of True. Since match() and search() return None when there is no match, you can test whether there was a match with a simple if statement:

    match = re.search(pattern, string)
    if match:
        process(match)

Match objects support the following methods and attributes:

class re.Match¶

    expand(template)

I mean, the "Match objects support the following methods and attributes:" looks wrong now that the following item is a class, not a list of methods and attributes.

@Jean-Abou-Samra In order to fix this, and to have a cleaner diff, can you please link using:

 :ref:`re.Pattern <re-objects>`

instead of:

:class:`re.Pattern`

? This would completly avoid touching the re.rst file, changing a 450-lines diff to a 10 lines diff.

Bonne soirée.

@Jean-Abou-Samra
Copy link
Contributor Author

@Jean-Abou-Samra Jean-Abou-Samra commented May 4, 2021

@JulienPalard I followed your suggestion and it should look cleaner now. (Also signed the CLA.)

The CI failure seems like an artifact of the renaming of the master branch yesterday. Let's see how it goes this time.

Copy link
Member

@JulienPalard JulienPalard left a comment

LGTM

@JulienPalard JulienPalard merged commit 0949029 into python:main May 4, 2021
12 checks passed
12 checks passed
@github-actions
Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
Azure Pipelines PR #20210504.42 succeeded
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@bedevere-bot
bedevere/issue-number Issue report skipped
@bedevere-bot
bedevere/news "skip news" label found
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented May 4, 2021

Thanks @Jean-Abou-Samra for the PR, and @JulienPalard for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒🤖

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 4, 2021

@JulienPalard: Please replace # with GH- in the commit message next time. Thanks!

@JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented May 4, 2021

(woops the GH- :D it's hard to ping-pong between repos with different rules )

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 4, 2021

GH-25900 is a backport of this pull request to the 3.10 branch.

miss-islington added a commit to miss-islington/cpython that referenced this pull request May 4, 2021
(cherry picked from commit 0949029)

Co-authored-by: Jean-Abou-Samra <37271310+Jean-Abou-Samra@users.noreply.github.com>
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 4, 2021

GH-25901 is a backport of this pull request to the 3.9 branch.

miss-islington added a commit that referenced this pull request May 4, 2021
(cherry picked from commit 0949029)

Co-authored-by: Jean-Abou-Samra <37271310+Jean-Abou-Samra@users.noreply.github.com>
miss-islington added a commit that referenced this pull request May 4, 2021
(cherry picked from commit 0949029)

Co-authored-by: Jean-Abou-Samra <37271310+Jean-Abou-Samra@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants