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

Misc: allow unknown args when finding prefix #38

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

aarani
Copy link
Contributor

@aarani aarani commented Oct 30, 2023

No description provided.

@knocte knocte merged commit 2737eea into nblockchain:master Oct 31, 2023
15 checks passed
@knocte
Copy link
Member

knocte commented Oct 31, 2023

I shouldn't have merged it because you were missing the why, sigh. Please write the why here.

@knocte
Copy link
Member

knocte commented Oct 31, 2023

ping

@aarani
Copy link
Contributor Author

aarani commented Oct 31, 2023

I shouldn't have merged it because you were missing the why, sigh. Please write the why here.

There's no reason for a code that's supposed to search arguments for a prefix, to limit or even care about other arguments.
We needed to accept other arguments besides "prefix" in one of our scripts and this code would throw and this commit fixes that.

@knocte
Copy link
Member

knocte commented Oct 31, 2023

There's no reason for a code that's supposed to search arguments for a prefix, to limit or even care about other arguments, this commit fixes that.

And why do you need it to not care about other arguments? Please state the ultimate why @aarani

@aarani
Copy link
Contributor Author

aarani commented Oct 31, 2023

There's no reason for a code that's supposed to search arguments for a prefix, to limit or even care about other arguments, this commit fixes that.

And why do you need it to not care about other arguments? Please state the ultimate why @aarani

edited*

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.

2 participants