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

Convert remap_restart to use argparse #291

Merged
merged 9 commits into from
Dec 8, 2022

Conversation

mathomp4
Copy link
Member

I saw that remap_restarts was using getopt but most developers recommend argparse for Python argument management.

This is currently just a rough first go at converting it. I'd need to talk with @weiyuan-jiang before finalizing it.

@mathomp4 mathomp4 added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Sep 12, 2022
@mathomp4 mathomp4 self-assigned this Sep 12, 2022
@mathomp4 mathomp4 marked this pull request as ready for review December 7, 2022 14:45
@mathomp4 mathomp4 requested review from a team as code owners December 7, 2022 14:45
@mathomp4 mathomp4 changed the title WIP: Convert remap_restart to use argparse Convert remap_restart to use argparse Dec 7, 2022
@mathomp4 mathomp4 added the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Dec 7, 2022
@github-actions
Copy link

github-actions bot commented Dec 7, 2022

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: Contingent - DNA, 0 diff

@github-actions
Copy link

github-actions bot commented Dec 7, 2022

Label error. Requires exactly 0 of: Contingent - DNA, Needs Lead Approval, Contingent -- Do Not Approve. Found: Contingent - DNA, 0 diff

@mathomp4
Copy link
Member Author

mathomp4 commented Dec 7, 2022

I'm going to add a blocker label to this until @weiyuan-jiang or someone more knowledgable than I can test the -c and -o options to make sure they work.

I'm fairly certain the questionary way works because I just tried it earlier to find some bugs in this. 😄

@weiyuan-jiang
Copy link
Contributor

Verified. -c and -o is still working @mathomp4

@mathomp4 mathomp4 removed the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Dec 7, 2022
@sdrabenh sdrabenh merged commit 026666b into main Dec 8, 2022
@sdrabenh sdrabenh deleted the feature/mathomp4/remap-argparse branch December 8, 2022 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants