Skip to content

bpo-45621: Small changes to mmap - #29247

Merged
tjguk merged 7 commits into
python:mainfrom
tjguk:issue45621-small-changes-to-mmap
Oct 29, 2021
Merged

bpo-45621: Small changes to mmap#29247
tjguk merged 7 commits into
python:mainfrom
tjguk:issue45621-small-changes-to-mmap

Conversation

@tjguk

@tjguk tjguk commented Oct 27, 2021

Copy link
Copy Markdown
Member

Following issue40915 a few small items still need to be addressed, mostly cosmetic / naming:

  • A comment should have been removed but wasn't
  • A comment should have been added but wasn't
  • The use of the string "TEST" should be avoided in tests particularly where there is a shared namespace and therefore the chance of a collision

https://bugs.python.org/issue45621

Comment thread Lib/test/test_mmap.py Outdated
Comment on lines +19 to +20
def random_tagname(length=10):
return "".join(random.choice(string.ascii_uppercase) for _ in range(length))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about including context in the name? For example:

tagname_prefix = f'python_{os.getpid()}_test_mmap'
def random_tagname(suffix_length=10):
    suffix = ''.join(random.choices(string.ascii_uppercase, k=suffix_length))
    return f'{tagname_prefix}_{suffix}'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, why not? (And thanks for the suggestion of random.choices: TIL)

@tjguk
tjguk merged commit 7bddd96 into python:main Oct 29, 2021
@tjguk
tjguk deleted the issue45621-small-changes-to-mmap branch October 29, 2021 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants