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

Exclude all classes in 'test-classes' from the serialization test #4483

Merged
merged 1 commit into from
Aug 6, 2018

Conversation

denis-anisimov
Copy link
Contributor

@denis-anisimov denis-anisimov commented Aug 4, 2018

This change is Reviewable

Copy link
Contributor

@ZheSun88 ZheSun88 left a comment

Choose a reason for hiding this comment

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

:

Reviewed 1 of 1 files at r1.
Reviewable status: all discussions resolved, 0 of 1 LGTMs obtained


flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java, line 148 at r1 (raw file):

                "com\\.vaadin\\.flow\\.templatemodel\\.BeanContainingBeans(\\$.*)?");
    }
    

extra spaces.

Copy link
Contributor

@ZheSun88 ZheSun88 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: 1 unresolved discussion, 0 of 1 LGTMs obtained

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: 1 unresolved discussion, 0 of 1 LGTMs obtained


flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java, line 148 at r1 (raw file):

Previously, ZheSun88 (Sun Zhe) wrote…

extra spaces.

So this is a blocker ?

@denis-anisimov denis-anisimov force-pushed the fix-serializable-test branch from d88fc82 to 29814cd Compare August 6, 2018 07:53
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: 1 unresolved discussion, 0 of 1 LGTMs obtained


flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java, line 148 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

So this is a blocker ?

Done.

Copy link
Contributor

@ZheSun88 ZheSun88 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: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale


flow-test-generic/src/main/java/com/vaadin/flow/testutil/ClassesSerializableTest.java, line 148 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Done.

in fact, it should not, but sometimes i got confused with the code reviewer's opinion.. maybe we should have some agreement on this kind of things.

@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 7 issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR ClassesSerializableTest.java#L164: Define and throw a dedicated exception instead of using a generic one. rule
  2. MAJOR ClassesSerializableTest.java#L289: Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. rule
  3. MAJOR ClassesSerializableTest.java#L289: Define and throw a dedicated exception instead of using a generic one. rule
  4. MAJOR ClassesSerializableTest.java#L303: Reduce the total number of break and continue statements in this loop to use at most one. rule
  5. MAJOR ClassesSerializableTest.java#L331: This block of commented-out lines of code should be removed. rule
  6. MAJOR ClassesSerializableTest.java#L432: Use the built-in formatting to construct this argument. rule
  7. INFO ClassesSerializableTest.java#L330: Complete the task associated to this TODO comment. rule

@ZheSun88 ZheSun88 merged commit 0940161 into master Aug 6, 2018
@ZheSun88 ZheSun88 deleted the fix-serializable-test branch August 6, 2018 08:09
@ZheSun88 ZheSun88 added this to the 1.1.0.beta2 milestone Aug 6, 2018
denis-anisimov pushed a commit that referenced this pull request Aug 6, 2018
)

* Exclude all classes in 'test-classes' from the serialization test
ZheSun88 pushed a commit that referenced this pull request Aug 6, 2018
)

* Exclude all classes in 'test-classes' from the serialization test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants