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

Add nym function #56

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Add nym function #56

merged 1 commit into from
Sep 6, 2023

Conversation

lhh
Copy link
Collaborator

@lhh lhh commented Aug 24, 2023

No description provided.

@lhh lhh force-pushed the master branch 2 times, most recently from d843489 to a3f0547 Compare August 24, 2023 18:40
@lhh lhh changed the title Add nym function, clean up tests Add nym function Aug 24, 2023
fuzzball81
fuzzball81 previously approved these changes Aug 24, 2023
Copy link
Member

@fuzzball81 fuzzball81 left a comment

Choose a reason for hiding this comment

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

Just a few non-blocking suggestions, otherwise looks good.

toolchest/strutil.py Show resolved Hide resolved
tests/test_strutil.py Outdated Show resolved Hide resolved
Copy link
Member

@jguiditta jguiditta left a comment

Choose a reason for hiding this comment

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

Overall, looks reasonable, just a few requests for tweaks.

toolchest/strutil.py Show resolved Hide resolved
tests/test_strutil.py Outdated Show resolved Hide resolved
fuzzball81
fuzzball81 previously approved these changes Aug 31, 2023
Copy link
Member

@jguiditta jguiditta left a comment

Choose a reason for hiding this comment

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

Other than the minor flake8 error, looks good to me! Thanks for making those tweaks, they help a lot.

toolchest/strutil.py Outdated Show resolved Hide resolved
Helper function for dealing with names while avoiding pesky
things like quoting, spaces, and case sensitivity
@fuzzball81 fuzzball81 merged commit 998daaf into release-depot:master Sep 6, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants