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

✨ Replace flag with pflag #109

Merged
merged 1 commit into from
Oct 23, 2023
Merged

✨ Replace flag with pflag #109

merged 1 commit into from
Oct 23, 2023

Conversation

aniruddha2000
Copy link
Collaborator

What type of PR is this?
/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #106

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squash commits
  • include documentation
  • add unit tests

main.go Outdated Show resolved Hide resolved
@janiskemper
Copy link
Collaborator

do we need to add pflag.CommandLine.AddGoFlagSet(flag.CommandLine) before parsing? I'm not sure...

@kranurag7
Copy link
Contributor

do we need to add pflag.CommandLine.AddGoFlagSet(flag.CommandLine) before parsing? I'm not sure...

Building the executable locally and invoking the same -h is working fine. I haven't tested the same on the tilt setup. @aniruddha2000 please test if you have a running tilt setup.

@aniruddha2000
Copy link
Collaborator Author

Well, I don't have any access to Hivelocity UI to see if devices are there or not, but I can clearly see that I have providerID in my k9s after creating a cluster by tilt, does that work?

@janiskemper
Copy link
Collaborator

janiskemper commented Oct 17, 2023

I think this is more a question of how flags work, not whether the operator works as a whole. But I guess you can proceed
EDIT: I think it doesn't harm to add it and we should do that in order to prevent issues in the future

janiskemper
janiskemper previously approved these changes Oct 17, 2023
kranurag7
kranurag7 previously approved these changes Oct 17, 2023
Signed-off-by: Aniruddha Basak <aniruddha.basak@syself.com>
@janiskemper janiskemper merged commit 875b7b5 into main Oct 23, 2023
6 of 7 checks passed
@janiskemper janiskemper deleted the ani/issues/106 branch October 23, 2023 08:59
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.

Switch to pflag package
3 participants