merge altsrc with main package

[asahasrabuddhe]

What type of PR is this?

• cleanup

What this PR does / why we need it:

This is one of the first goals for the v3 release (via https://github.com/urfave/cli/issues/833#issue-476467837). This helps condense the public API by reducing two exposed packages from before into a single package.

Release Notes

- Remove `altsrc` package
- Add `FlagInputSourceExtension` interface to allow a value to be set on existing parsed flags through alternate sources like `JSON` or `TOML` etc.

[codecov[bot]]

Codecov Report

:exclamation: No coverage uploaded for pull request base (v3-development-master@c5f50db). Click here to learn what that means. The diff coverage is 64.18%.

@@                   Coverage Diff                    @@
##             v3-development-master    #1058   +/-   ##
========================================================
  Coverage                         ?   67.77%           
========================================================
  Files                            ?       31           
  Lines                            ?     2647           
  Branches                         ?        0           
========================================================
  Hits                             ?     1794           
  Misses                           ?      741           
  Partials                         ?      112

Impacted Files	Coverage Δ
errors.go	87.5% <ø> (ø)
category.go	87.5% <ø> (ø)
json_source_context.go	12.37% <0%> (ø)
app.go	78.71% <100%> (ø)
yaml_file_loader.go	42.85% <100%> (ø)
flag.go	85.57% <100%> (ø)
flag_string.go	87.5% <100%> (ø)
flag_duration.go	73.07% <100%> (ø)
flag_int_slice.go	76.82% <100%> (ø)
flag_string_slice.go	81.08% <100%> (ø)
... and 16 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c5f50db...120695c. Read the comment docs.

[rliebz]

Motivation, for context: https://github.com/urfave/cli/pull/907#issuecomment-578426446

[rliebz]

This might be something to balance against the goal of making the project more modular, re: https://github.com/urfave/cli/issues/1055. Two of our three dependencies, github.com/BurntSushi/toml and gopkg.in/yaml.v2, come from the altsrc package.

[asahasrabuddhe]

This PR doesn't intend to make the project modular. It simply exists to integrate the altsrc into the main package. To make the library modular is the last thing in my plan when I am sure that the core features intended for v3 are already in the development branch.

Once done, I can think of functionality that can be offloaded into a separate module and downloaded on demand by a user who wants it.

[rliebz]

Sorry, I may have phrased that poorly.

This change seems to move in the opposite direction of modularity. We're taking functionality that comes with dependencies such as yaml and toml-parsing and moving that from a separate package into the cli package. Not that there isn't reason to do this—modularity obviously isn't our only goal. If modularity is one of our goals, though, I wanted to flag that we're bringing more dependencies into our core package by making this change, which might make it harder to stay modular in the future, unless we have a specific path forward on that front.

[asahasrabuddhe]

I agree with you @rliebz. Apologies if I came too strongly there. What I wanted to say is I just wanted all the proposed changes together in the development branch to see how they work out together. Once the features are frozen, I would proceed with a cleanup and extract such optional components into separate modules which can then be imported by users when they want to use them.

[coilysiren]

I @asahasrabuddhe @rliebz I think you both just made a very strong argument for us including a "module system" in the v3 release!

[coilysiren]

I agree with @asahasrabuddhe's proposal of merging altsrc into cli, and then moving it out again later - after the module system is in.

[stale[bot]]

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[bot]]

Closing this as it has become stale.

[stale[bot]]

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

[asahasrabuddhe]

I'd like us all to re consider this PR and we can then probably consider moving forward in the direction of a modular v3

[asahasrabuddhe]

@urfave/cli

[rliebz]

There have been a number of changes to the master branch since this was last worked on, and in particular, a number of fixes applied to the altsrc package. I think we're not seeing any conflicts now because v3-development-master has lagged behind, but I want to make sure that we aren't losing out on changes, especially in a ~3000 line diff.

Not sure what the best way to proceed is here—potentially start by getting v3-development-master up to date with master?

[stale[bot]]

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.

[coilysiren]

👀

[stale[bot]]

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

[stale[bot]]

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.

[meatballhat]

👀

[dearchap]

@asahasrabuddhe Can you fix the merge conflicts ?

[coilysiren]

OMG ITS HAPPENING!?!?!

[dearchap]

I'm going to close this issue since we are moving altsrc to a separate repo