Skip to content

Rollback package.json if plugin installation fails #2536

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 1 commit into from
Feb 15, 2017

Conversation

petekanev
Copy link
Contributor

Implement rollback of package.json contents of project if npm installation fails for whatever reason during installation of a NS plugin or platform.

Addresses #2487

master-based PR: #2531

@petekanev petekanev added this to the 2.5.1 milestone Feb 14, 2017
@petekanev petekanev self-assigned this Feb 14, 2017
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

after fixing comment and green build

@@ -52,7 +50,7 @@ export class NpmInstallationManager implements INpmInstallationManager {
return this.installCore(packageToInstall, pathToSave, version, dependencyType).wait();
} catch (error) {
this.$logger.debug(error);
this.$errors.fail("%s. Error: %s", NpmInstallationManager.NPM_LOAD_FAILED, error);
this.$errors.fail("%s", error);
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be just throw error.
$errors.fail is usable only when we change the error itself, which is not the current case.

throw error when npm-installation-manager install fails
@petekanev petekanev force-pushed the pete/plugin-install-fail-rollback-251 branch from d94059f to 5f44f12 Compare February 14, 2017 13:51
@rosen-vladimirov rosen-vladimirov merged commit b6823f7 into release Feb 15, 2017
@rosen-vladimirov rosen-vladimirov deleted the pete/plugin-install-fail-rollback-251 branch February 15, 2017 07:39
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