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

Added script to generate altsrc flag definitions #1244

Closed
wants to merge 1 commit into from

Conversation

ipostelnik
Copy link

@ipostelnik ipostelnik commented Feb 12, 2021

Added script to generate altsrc flag definitions, restricted list of types to the ones that provide implementation of FlagInputSourceExtension.

What type of PR is this?

  • bug
  • cleanup

What this PR does / why we need it:

  • The original program to generate altsrc flags is nowhere to be found, so replaced with fg.py.
  • The types of altsrc flags are now limited to the subset that provide FlagInputSourceExtension implementation and have matching methods in InputSourceContext interface. Removed types where never updated from the input source.

Which issue(s) this PR fixes:

Testing

Existing unit tests

Release Notes

Removed altsrc flag definitions for types that do not implement FlagInputSourceExtension. These types did not have their values loaded from the configuration file.


@ipostelnik ipostelnik requested a review from a team as a code owner February 12, 2021 16:05
@ipostelnik ipostelnik requested review from coilysiren and asahasrabuddhe and removed request for a team February 12, 2021 16:05
…t sources.

- Updated unit tests for atlsrc handling to match expected usage via cli.App. Previously, the tests tried to mock cli.Context which caused differences in behavior.
- The types of altsrc flags are now limited to the subset that provide FlagInputSourceExtension implementation and have matching methods in InputSourceContext interface. Removed types where never updated from the input source.
- Added script to generate altsrc flag definitions, restricted list of types to the ones that provide implementation of FlagInputSourceExtension. The original program to generate altsrc flags is nowhere to be found, so replaced with fg.py.
@coilysiren coilysiren requested review from a team and removed request for coilysiren and asahasrabuddhe April 23, 2021 19:44
@stale
Copy link

stale bot commented Jul 22, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Jul 22, 2021
@stale
Copy link

stale bot commented Aug 22, 2021

Closing this as it has become stale.

@stale stale bot closed this Aug 22, 2021
@meatballhat meatballhat reopened this Apr 19, 2022
@meatballhat meatballhat added area/v2 relates to / is being considered for v2 and removed status/stale stale due to the age of it's last update labels Apr 21, 2022
@meatballhat meatballhat changed the title Added script to generate altsrc flag definitions, restricted list of … Added script to generate altsrc flag definitions Apr 21, 2022
@meatballhat meatballhat added kind/bug describes or fixes a bug kind/cleanup describes internal cleanup / maintaince labels Apr 21, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 21, 2022
@meatballhat meatballhat changed the base branch from master to main April 21, 2022 22:04
@meatballhat meatballhat added the status/conflicts contains merge conflicts label Apr 26, 2022
@meatballhat
Copy link
Member

@ipostelnik Sorry for the long delay! 😭 Any chance you're interested in refreshing this work, especially in light of the most recent round of flag generation tooling?

@meatballhat
Copy link
Member

@dearchap if you're interested in seeing this work moved forward, I think I would prefer the use of the more recently-(re)introduced internal/genflags. WDYT?

@dearchap
Copy link
Contributor

@meatballhat That should be fine. @ipostelnik Are you up to resolve the conflicts ?

@dearchap dearchap added the status/waiting-for-response Waiting for response from original requester label Sep 26, 2022
@dearchap
Copy link
Contributor

PR #1553 Does this on latest and uses the internal gen flag program.

@dearchap dearchap closed this Oct 29, 2022
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/bug describes or fixes a bug kind/cleanup describes internal cleanup / maintaince status/conflicts contains merge conflicts status/waiting-for-response Waiting for response from original requester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants