Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Improvement to get_argument#2939

Merged
mergify[bot] merged 1 commit intoinstructlab:maininstructlab/instructlab:mainfrom
reidliu41:improve-get_argumentreidliu41/instructlab:improve-get_argumentCopy head branch name to clipboard
Jan 20, 2025
Merged

Improvement to get_argument#2939
mergify[bot] merged 1 commit intoinstructlab:maininstructlab/instructlab:mainfrom
reidliu41:improve-get_argumentreidliu41/instructlab:improve-get_argumentCopy head branch name to clipboard

Conversation

@reidliu41
Copy link
Contributor

Based on #2927 (comment)

$ python test-test.py
prefix:  --foo
args:  ['--foo', '4']
flag:  True
value:  4

$ python test-test.py
prefix:  --foo
args:  ['--foo', '4', '--foo']
flag:  True
value:  None

$ python test-test.py
prefix:  --foo
args:  ['--foo', '--foo']
flag:  True
value:  None

$ python test-test.py
prefix:  --foo
args:  ['--foo', '4', '--foo=2']
flag:  True
value:  2

$ python test-test.py
prefix:  --foo
args:  ['foo']
flag:  False
value:  None

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the
    conventional commits.
  • Changelog updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Functional tests have been added, if necessary.
  • E2E Workflow tests have been added, if necessary.

Signed-off-by: reid_liu <guliu@redhat.com>
@reidliu41 reidliu41 requested a review from danmcp January 19, 2025 00:21
Copy link
Contributor

@danmcp danmcp left a comment

Choose a reason for hiding this comment

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

Great change, thanks for the PR!

Would probably be good to have a test for this logic (separate PR would be fine).

@mergify mergify bot added the one-approval PR has one approval from a maintainer label Jan 19, 2025
@mergify mergify bot merged commit 271ea1b into instructlab:main Jan 20, 2025
27 checks passed
@mergify mergify bot removed the one-approval PR has one approval from a maintainer label Jan 20, 2025
mergify bot added a commit that referenced this pull request Jan 21, 2025
For #2939 (review)

**Checklist:**

- [ ] **Commit Message Formatting**: Commit titles and messages follow guidelines in the
  [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/#summary).
- [ ] [Changelog](https://github.com/instructlab/instructlab/blob/main/CHANGELOG.md) updated with breaking and/or notable changes for the next minor release.
- [ ] Documentation has been updated, if necessary.
- [ ] Unit tests have been added, if necessary.
- [ ] Functional tests have been added, if necessary.
- [ ] E2E Workflow tests have been added, if necessary.



Approved-by: danmcp

Approved-by: nathan-weinberg
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.