Skip to content

Multiple remotes, TOML configuration#99

Merged
bobheadxi merged 14 commits into
masterfrom
rob/#41-multi-remotes
Mar 3, 2018
Merged

Multiple remotes, TOML configuration#99
bobheadxi merged 14 commits into
masterfrom
rob/#41-multi-remotes

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Feb 28, 2018

Copy link
Copy Markdown
Member

🎟️ Ticket(s):


👷 Changes

  • Revamped client.Config struct
  • New TOML configuration file format, stored as .inertia.toml
  • Multiple remotes!
    • each has its own set of authentication and daemon settings
inertia = "v0.0.1-95-g1b78ab5"
project = "inertia-deploy-test"

[[remote]]
    name = "test"
    ...
    [remotes.daemon]
        port = "8080"
        ...

[[remote]]
    name = "test2"
    ...
    [remotes.daemon]
        port = "80801"
        ...
  • List remote details!
  • Remove remotes!
  • Reset Inertia configuration!
  • Versions!
  • Some tests for various funcs, unfortunately testing the main package seems to have dropped our coverage significantly 😢

🔦 Testing Instructions

inertia init
inertia remote add local
inertia remote add gcloud

inertia remote ls -v
inertia remote rm gcloud
inertia remote ls

Works for the deploy commands too:

inertia local up
inertia gcloud up

😁
Some other things:

inertia --version
inertia reset

* Initial support for multiple remotes
* Switch to TOML configuration file
* Configuration struct redesign
* Rename client.init to client.config
@bobheadxi bobheadxi added the pr: finalized needs review and final approval label Feb 28, 2018
@bobheadxi bobheadxi requested a review from bfbachmann February 28, 2018 17:34

@PiggySpeed PiggySpeed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Took some time to figure out what was happening, but it looks good.

Comment thread client/config.go Outdated
config.Remotes = append(config.Remotes, remote)
}

// RemoveRemote removes remote at with given name

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

*removes remote with given name

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

😅

Comment thread init.go
if err != nil {
log.Fatal("invalid response - aborting")
}
if response != "y" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it standard behaviour for cli tools to treat all inputs except "y" as a "n"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not too sure actually. Personally I think since this is a destructive action, any command other than y can safely be assumed to be a "no"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, that's fine with me then!

@bobheadxi bobheadxi merged commit fb3ff9b into master Mar 3, 2018
@bobheadxi bobheadxi deleted the rob/#41-multi-remotes branch March 3, 2018 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: finalized needs review and final approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants