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

Unified Command Line Interface #495

Merged
merged 8 commits into from
Sep 10, 2021
Merged

Unified Command Line Interface #495

merged 8 commits into from
Sep 10, 2021

Conversation

cimnine
Copy link
Contributor

@cimnine cimnine commented Sep 3, 2021

Summary

Both modules (restic and operator) are now configured via urfave/cli's Flags. All the flags' values can be still be defined via their respective ENV variables.

The advantage is that both modules build upon the same mechanism to configuration. Also urface/cli does some basic parsing and it has built-in support for required arguments / ENV vars.
Finally, it reduces one dependency.

Closes #477

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking,
    as they show up in the changelog
  • Update the documentation.
  • Update tests.
  • Link this PR to related issues.

@cimnine cimnine added the change Generic change that is neither a fix or feature label Sep 3, 2021
@cimnine cimnine added this to the Wrestic Integration milestone Sep 3, 2021
@cimnine cimnine self-assigned this Sep 3, 2021
@cimnine cimnine force-pushed the UnifiedCommandLineInterface branch 7 times, most recently from 0b64586 to c3a5b8b Compare September 8, 2021 13:03
Previously, the code was clustered with os.GetEnv and os.LookupEnv calls.
Not there is a central source and the configuration is built when the program starts.
The configuration can be checked when the program starts, adhering to the 'Fail Fast' principle.
@cimnine cimnine marked this pull request as ready for review September 8, 2021 14:57
Copy link
Contributor

@ccremer ccremer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

cmd/operator/main.go Show resolved Hide resolved
restic/cfg/config.go Outdated Show resolved Hide resolved
restic/cfg/config.go Outdated Show resolved Hide resolved
@cimnine cimnine force-pushed the UnifiedCommandLineInterface branch 2 times, most recently from bb4ba8e to 630443b Compare September 10, 2021 07:32
docs/docs.mk Show resolved Hide resolved
operator/cfg/config.go Outdated Show resolved Hide resolved
Copy link
Contributor

@bastjan bastjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@cimnine cimnine merged commit 69b7692 into master Sep 10, 2021
@cimnine cimnine deleted the UnifiedCommandLineInterface branch September 10, 2021 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change Generic change that is neither a fix or feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unifi CLI of backup and restic modules of k8up
3 participants