Skip to content

Linux compatibility#99

Merged
febbraro merged 6 commits into
developfrom
feature/linux-compat
Oct 27, 2017
Merged

Linux compatibility#99
febbraro merged 6 commits into
developfrom
feature/linux-compat

Conversation

@febbraro
Copy link
Copy Markdown
Member

No description provided.

Comment thread cli/commands/dashboard.go

const (
dashboardContainerName string = "outrigger-dashboard"
dashboardImageName string = "outrigger/dashboard:latest"
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.

I made these lower camel case to keep them as private constants. Do we have a use case to export them?

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.

Did not mean to make them public (by convention), was just looking at some approaches. i can revert that.

Comment thread cli/commands/dashboard.go Outdated

func (cmd *Dashboard) Run(ctx *cli.Context) error {
if cmd.machine.IsRunning() {
if cmd.machine.IsRunning() || runtime.GOOS == "linux" {
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.

Nice catch.

Comment thread cli/commands/data_backup.go Outdated
}

func (cmd *DataBackup) Run(c *cli.Context) error {
if runtime.GOOS == "linux" {
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.

Given the need to add runtime and check on Linux everywhere, I wonder if we shouldn't add a util/feature_detection.go now and define util.CanRunNative() as a check for linux.

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.

util.NativeDockerSupport() ?

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.

In anticipation of other things to detect, util.SupportsNativeDocker().

util.SupportsFileSync() would currently exclude windows without bash right?


func (cmd *DataRestore) Run(c *cli.Context) error {
if runtime.GOOS == "linux" {
return cmd.Success("Data Restore is not needed on Linux, please unarchive any data directly")
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.

This makes sense to me, but if we ever expanded on the data backup to include volume wrangling this would change.

Copy link
Copy Markdown
Contributor

@grayside grayside left a comment

Choose a reason for hiding this comment

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

Looks good to me, just needs the go fmt fixup.

@febbraro febbraro merged commit b42d723 into develop Oct 27, 2017
@febbraro febbraro deleted the feature/linux-compat branch October 27, 2017 03:24
@grayside grayside added this to the v2.0 milestone Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants