Skip to content

Refactored config to not use viper singleton instance #1027

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

Merged
merged 6 commits into from
Oct 14, 2020

Conversation

silvanocerza
Copy link
Contributor

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce?
    Refactors how the configs are initialized and fixes config init.
  • What is the current behavior?
    Configs are managed using the viper package singleton, config init command reads from existing settings when initializing a new config file.
  • What is the new behavior?
    Configs are not managed anymore with the viper package singleton but using a global variable internal to the configuration package of the arduino-cli.
    The config init command now initializes new config files using the default values without reading from existing files.
  • Does this PR introduce a breaking change?
    Yes.
  • Other information:
    None.

See how to contribute

@silvanocerza silvanocerza added kind/bug topic: documentation Related to documentation for the project status: on hold Do not proceed at this time labels Oct 12, 2020
@silvanocerza silvanocerza self-assigned this Oct 12, 2020
@silvanocerza silvanocerza force-pushed the scerza/config-refactoring branch from f57bcae to fbc09e0 Compare October 12, 2020 16:15
@silvanocerza silvanocerza force-pushed the scerza/config-refactoring branch from fbc09e0 to cd1d472 Compare October 12, 2020 16:18
Copy link

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

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

Tested and it finally works

@silvanocerza silvanocerza removed the status: on hold Do not proceed at this time label Oct 13, 2020
Copy link
Member

@cmaglie cmaglie left a comment

Choose a reason for hiding this comment

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

I've added some more commits, so I approve all the rest :-)
@silvanocerza if you are ok with my changes, IMHO this is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Related to documentation for the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants