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

CHE-398: Add ability to create local machine snapshots without registry #1351

Merged
merged 1 commit into from
May 27, 2016

Conversation

akorneta
Copy link
Contributor

@garagatyi, @skabashnyuk, @evoevodin

}
}

@VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

why VisibleForTesting and not only protected access instead of package access ?
also there is no test calling this method so I don't see why it should be "VisibleForTesting"

@akorneta akorneta force-pushed the CHE-398 branch 2 times, most recently from ff5effb to 863538d Compare May 25, 2016 14:58
@@ -150,3 +149,6 @@ workspace.runtime.auto_snapshot=false
# During the start of the workspace automatically restored it from a snapshot if the value is {true},
# otherwise just creates the new workspace.
workspace.runtime.auto_restore=false
# Allows to use registry for workspace snapshots, you should set this property in {true},
# otherwise workspace snapshots would be saved locally.
workspace.snapshot_use_registry=false
Copy link
Contributor

Choose a reason for hiding this comment

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

missing end of line

@akorneta akorneta force-pushed the CHE-398 branch 3 times, most recently from 5514a93 to 2cd75bc Compare May 26, 2016 09:04
final String digest = saveInRegistry(owner, repository, tag);
return new DockerInstanceKey(repository, tag, registry, digest);
} else {
createImage(owner, repository, tag);

Choose a reason for hiding this comment

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

I think it is better to name that method in the same way as saveInRegistry - saveLocallly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 ok

@akorneta akorneta force-pushed the CHE-398 branch 2 times, most recently from 4000ffd to 48e798d Compare May 26, 2016 12:23
@garagatyi
Copy link

LGTM

@voievodin
Copy link
Contributor

ok

@skabashnyuk
Copy link
Contributor

ок

@akorneta akorneta merged commit 45251ee into master May 27, 2016
@akorneta akorneta deleted the CHE-398 branch May 27, 2016 08:26
@codenvy-ci
Copy link

Build # 732 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/732/ to view the results.

This pull request was closed.
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.

6 participants