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

What is the purpose of normalizeFlags? #1929

Closed
fiatjaf opened this issue Jun 26, 2024 · 3 comments · Fixed by #1941
Closed

What is the purpose of normalizeFlags? #1929

fiatjaf opened this issue Jun 26, 2024 · 3 comments · Fixed by #1941
Labels
area/v2 relates to / is being considered for v2 kind/question someone asking a question status/triage maintainers still need to look into this

Comments

@fiatjaf
Copy link

fiatjaf commented Jun 26, 2024

I read the code but my understanding of it doesn't match what happens when I remove that function entirely.

It looks like the only things it does in practice is to disallow using the long form and the short form alias of the same flag in a same call.

Is there a reason why that should be disallowed? Or is that a necessity due to some constraint I am not seeing?

@fiatjaf fiatjaf added area/v2 relates to / is being considered for v2 kind/question someone asking a question status/triage maintainers still need to look into this labels Jun 26, 2024
@dearchap
Copy link
Contributor

@fiatjaf Thats one of the reasons. The other reason is that the flag aliases values are duplicated into the set for retrieval but set.Value function.

@dearchap
Copy link
Contributor

@fiatjaf looking at it further it appears that the function is redudant. It is a holdover from an early v2 codebase. I have removed in v3.

@fiatjaf
Copy link
Author

fiatjaf commented Jun 30, 2024

Thank you, this is good news!

@fiatjaf fiatjaf closed this as completed Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/question someone asking a question status/triage maintainers still need to look into this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants