Skip to content

gh-128427: Add uuid.NIL and uuid.MAX - #128429

Merged
hugovk merged 7 commits into
python:mainfrom
ngnpope:nil-max-uuid
Jan 27, 2025
Merged

gh-128427: Add uuid.NIL and uuid.MAX#128429
hugovk merged 7 commits into
python:mainfrom
ngnpope:nil-max-uuid

Conversation

@ngnpope

@ngnpopengnpope commented Jan 2, 2025•
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

Adds constants for Nil and Max UUID formats as per RFC 9562.

📚 Documentation preview 📚: https://cpython-previews--128429.org.readthedocs.build/

@bedevere-appbedevere-appBot mentioned this pull request Jan 2, 2025
Closed
Wulian233
Wulian233 reviewed Jan 2, 2025
View reviewed changes
Comment threadDoc/whatsnew/3.14.rst Outdated
@picnixz
picnixz self-requested a review January 2, 2025 23:11
picnixz
picnixz reviewed Jan 3, 2025
View reviewed changes
Comment threadDoc/library/uuid.rst Outdated
Comment threadDoc/library/uuid.rst Outdated
Comment threadDoc/whatsnew/3.14.rst
Comment threadLib/test/test_uuid.py Outdated
Comment threadLib/uuid.py
Comment threadMisc/NEWS.d/next/Library/2025-01-02-20-34-04.gh-issue-128427.onPoQZ.rst Outdated
Comment threadDoc/library/uuid.rst
Comment threadDoc/library/uuid.rst
@picnixz

picnixz commented Jan 3, 2025•
edited
Loading

Copy link
Copy Markdown
Member

By the way, avoid force-pushing in the future as it makes reviews harder; we eventually squash commits and write our own commit message (force-pushing is accepted in some cases only but those are quite rare (like fixing up a really wrong commit that should not be seen at all, or changing the committer's email/name to due CLA reasons)).

ngnpope
ngnpope commented Jan 3, 2025
View reviewed changes
Comment threadLib/test/test_uuid.py Outdated
picnixz
picnixz reviewed Jan 5, 2025
View reviewed changes

@picnixzpicnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I forgot about the version and variant fields which I don't know what to do with.

Comment threadLib/test/test_uuid.py Outdated
Comment threadLib/test/test_uuid.py Outdated
Comment threadLib/test/test_uuid.py
@ngnpope

This comment was marked as resolved.

Sign in to view
@picnixz

This comment was marked as resolved.

Sign in to view
picnixz
picnixz approved these changes Jan 6, 2025
View reviewed changes

@picnixzpicnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just two nits and LGTM once a core dev allows it.

Comment threadLib/test/test_uuid.py Outdated
Comment threadLib/test/test_uuid.py Outdated
@picnixzpicnixz added the type-feature A feature request or enhancement label Jan 13, 2025
@picnixz
picnixz self-requested a review January 13, 2025 10:07
hugovk
hugovk approved these changes Jan 13, 2025
View reviewed changes

@hugovkhugovk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, just a couple of docs suggestions.

Comment threadDoc/library/uuid.rst
.. data:: NIL

A special form of UUID that is specified to have all 128 bits set to zero
according to :rfc:`RFC 9562, §5.9 <9562#section-5.9>`.

@hugovkhugovkJan 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Linking directly to the section is helpful, perhaps we only need to mention the RFC number here? If so, we could update the same thing for uuid8 above.

Suggested change
according to :rfc:`RFC 9562, §5.9 <9562#section-5.9>`.
according to :rfc:`RFC 9562 <9562#section-5.9>`.

Comment threadDoc/library/uuid.rst
.. data:: MAX

A special form of UUID that is specified to have all 128 bits set to one
according to :rfc:`RFC 9562, §5.10 <9562#section-5.10>`.

@hugovkhugovkJan 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
according to :rfc:`RFC 9562, §5.10 <9562#section-5.10>`.
according to :rfc:`RFC 9562 <9562#section-5.10>`.

encukou
encukou approved these changes Jan 27, 2025
View reviewed changes

@encukouencukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it works better with the section number in the text.

@hugovk

hugovk commented Jan 27, 2025

Copy link
Copy Markdown
Member

Sure, let's merge.

@ngnpope, thank you your contribution!

@hugovk
hugovk merged commit 8ec76d9 into python:mainJan 27, 2025
@ngnpope
ngnpope deleted the nil-max-uuid branch January 27, 2025 18:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukouencukouencukou approved these changes
@hugovkhugovkhugovk approved these changes
@picnixzpicnixzpicnixz approved these changes
+1 more reviewer
@Wulian233Wulian233Wulian233 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type-featureA feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ngnpope@picnixz@hugovk@encukou@Wulian233