Skip to content

Conversation

@imneonizer
Copy link
Contributor

@imneonizer imneonizer commented May 24, 2021

A more pythonic way to create directories if doesn't exists.

A more pythonic way to create directories if doens't exists.
@roclark roclark self-assigned this May 24, 2021
@roclark roclark self-requested a review May 24, 2021 15:13
@roclark roclark added the enhancement New feature or request label May 24, 2021
@roclark roclark added this to the Release 1.1.3 milestone May 24, 2021
@roclark
Copy link
Member

roclark commented May 24, 2021

Hey @imneonizer, thanks for putting this together! This looks great and I'd be happy to merge it, but do you mind putting a "Signed-Off-By: Name email" at the end of your commit? Something like:

Fail safe makedirs

A more Pythonic ...

Signed-Off-By: First Last <[email protected]>

This just helps us ensure all contributors acknowledge the Contributors License Agreement for the repo. Once that's done and the tests pass, I'd be happy to merge!

@imneonizer
Copy link
Contributor Author

@roclark Thank you for the info, I have updated the commit message, hope this works.

@roclark
Copy link
Member

roclark commented May 25, 2021

Thanks for following up @imneonizer! I believe I may have misspoke earlier - I meant to say the commit message I used as an example above should be embedded in your actual commit as opposed to the pull request on GitHub. That way, your Signed-Off-By: ... can be seen when someone does a git log to view all commits/messages from a command line. This can be done with a git commit --amend or similar to update the existing commit, followed by a git push --force.

Also, it appears the CI tests are currently failing with an extra whitespace. I can point this out in the code if that helps. Preferably, the updated commit message and the removal of the extra whitespace will be done in the same, single commit.

Thanks again for taking the time for this! Please let me know if there are any further questions.

Signed-Off-By: Nitin Rai [email protected]
@imneonizer
Copy link
Contributor Author

@roclark
Thanks for such a detailed explanation, I have made the changes.
This is my first PR in open-source projects at Nvidia, really excited to see it merged!!

@roclark
Copy link
Member

roclark commented May 27, 2021

Hey @imneonizer, sorry for not getting to this yesterday! Thanks for cleaning things up here - this looks good to me and I am happy to merge it! I will do a squash commit to combine your two commits into one and include it with the main branch. Thanks for creating this and I'm happy to play my little part in your first PR with NVIDIA! 😃 I welcome any future PRs you may have as well!

@roclark roclark merged commit e1d6f75 into NVIDIA:main May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants