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

Do clean up in case the shrink-wrap versions are different #6001

Merged
merged 4 commits into from
Jul 2, 2019

Conversation

denis-anisimov
Copy link
Contributor

@denis-anisimov denis-anisimov commented Jun 28, 2019

Fixes #5967


This change is Reviewable

@project-bot project-bot bot added this to Review in progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jun 28, 2019
}

private String getShrinkWrapVersion(JsonObject packageJson)
throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

MINOR Remove the declaration of thrown exception 'java.io.IOException', as it cannot be thrown from method's body. rule

@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 2 issues

  • MAJOR 1 major
  • MINOR 1 minor

Watch the comments in this conversation to review them.

private void cleanUp() throws IOException {
File packageLock = getPackageLock();
if (packageLock.exists()) {
if (!packageLock.delete()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

MAJOR Merge this if statement with the enclosing one. rule

@ujoni ujoni self-assigned this Jul 1, 2019
Copy link
Contributor

@ujoni ujoni left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)


flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java, line 125 at r1 (raw file):

    }

    private void ensureReleaseVersion(JsonObject dependencies)

I am unable to build flow, but the code looks good.
The only addition I'd like is a comment on this function explaining what it does and which files are potentially affected.

Copy link
Contributor Author

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @denis-anisimov and @ujoni)


flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskUpdatePackages.java, line 125 at r1 (raw file):

Previously, ujoni (Joni) wrote…

I am unable to build flow, but the code looks good.
The only addition I'd like is a comment on this function explaining what it does and which files are potentially affected.

Done.

Copy link
Contributor

@ujoni ujoni left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: 2 unresolved discussions, 1 of 1 LGTMs obtained (waiting on @denis-anisimov)

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Review in progress to Reviewer approved Jul 2, 2019
Copy link
Contributor Author

@denis-anisimov denis-anisimov left a comment

Choose a reason for hiding this comment

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

Dismissed @vaadin-bot from 2 discussions.
Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

@denis-anisimov denis-anisimov merged commit 1fffc42 into master Jul 2, 2019
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Done - pending release Jul 2, 2019
@denis-anisimov denis-anisimov deleted the 5967-clean-on-version-update branch July 2, 2019 06:33
@ujoni ujoni added this to the 2.0.3 milestone Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

Projects get broken for no good reason
3 participants