-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
First cut at using generics for base flag types #1512
Conversation
iirc using generics lessens type checking, and moves errors to runtime, but it might work as a wrapper to a flag lib itself I have a weekend, I'll try to review this or work on harg.. |
What were you experimenting with |
Not sure, it wasnt a direct import |
I can't yet put my finger on it, but be aware, that multiple interface typing (
edit: this seems to not be a problem in this source, it's the hacks with stdlib/flag, that made me feel it edit2: I think I don't have much more to add atm, I'd rather go work on harg, or something else on my list. |
4a10d22
to
e926dde
Compare
jfyi generics are not the keyword here, what you/we are doing is implementing/using (generic) interfaces |
I'd move flags to a separate subpackage (a subdirectory), this would most importantly isolate it, but also enable possible external uses. |
30261ac
to
2737f34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nits only! This is very exciting! 🎉
Co-authored-by: Dan Buch <dan@meatballhat.com>
Co-authored-by: Dan Buch <dan@meatballhat.com>
Co-authored-by: Dan Buch <dan@meatballhat.com>
Co-authored-by: Dan Buch <dan@meatballhat.com>
Co-authored-by: Dan Buch <dan@meatballhat.com>
Co-authored-by: Dan Buch <dan@meatballhat.com>
025a2f5
to
e2e3aa0
Compare
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
(REQUIRED)
Use generics all the way
Which issue(s) this PR fixes:
(REQUIRED)
Connected to #833
Special notes for your reviewer:
(fill-in or delete this section)
Testing
(fill-in or delete this section)
Release Notes
(REQUIRED)