search

vaadin / patient-portal-demo-flow

0 stars 0 forks source link

Verify PRs with IT tests #88

Closed denis-anisimov closed 6 years ago

denis-anisimov commented 6 years ago

This change is Reviewable

ZheSun88 commented 6 years ago

Review status: 0 of 13 files reviewed at latest revision, all discussions resolved.

a discussion (no related file): what happened with the lumo annotation? I saw we have a few commented out session on it. do we leave that for testing or something else?

Is the purpose of this PR to remove Travis and start using other CI (teamCity) for validation?

pom.xml, line 451 at r1 (raw file):

     <test.excludegroup></test.excludegroup>

what kind of usage is this? nothing got defined?

Comments from Reviewable

denis-anisimov commented 6 years ago

Review status: 0 of 13 files reviewed at latest revision, 2 unresolved discussions.

a discussion (no related file):

Previously, ZheSun88 (Sun Zhe) wrote…
what happened with the `lumo` annotation? I saw we have a few commented out session on it. do we leave that for testing or something else? Is the purpose of this PR to remove Travis and start using other CI (teamCity) for validation?

Lumo is not needed since it should be applied automatically. I have a bunch of crappy behavior locally with this application so I've commented out Lumo in pom.xml as well. But I will uncomment it.

Travis doesn't do any validation currently. It just compiles. Yes, the main purpose is to get a validation on TC. And at the moment the IT tests are completely broken. So it returns them back.

Comments from Reviewable

denis-anisimov commented 6 years ago

Review status: 0 of 13 files reviewed at latest revision, 2 unresolved discussions.

pom.xml, line 451 at r1 (raw file):

Previously, ZheSun88 (Sun Zhe) wrote…
> ``` > > ``` what kind of usage is this? nothing got defined?

The purpose is to define an empty property. It's intended to be empty in all-tests profile. It's not empty in the default profile which allows to exclude some tests via category.

Comments from Reviewable

ZheSun88 commented 6 years ago

Reviewed 13 of 13 files at r1. Review status: all files reviewed at latest revision, 2 unresolved discussions.

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…
`Lumo` is not needed since it should be applied automatically. I have a bunch of crappy behavior locally with this application so I've commented out `Lumo` in `pom.xml` as well. But I will uncomment it. Travis doesn't do any validation currently. It just compiles. Yes, the main purpose is to get a validation on TC. And at the moment the IT tests are completely broken. So it returns them back.

Okay, if lumo is not needed, can we remove them instead of un-comment it?

Comments from Reviewable

denis-anisimov commented 6 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.

a discussion (no related file):

Previously, ZheSun88 (Sun Zhe) wrote…
Okay, if `lumo` is not needed, can we remove them instead of un-comment it?

I don't need Lumo because it breaks my project. Technically Lumo should be applied. There is no need to specify it explicitly. Since it's applied implicitly if it in the classpath. So there is no need in annotation but it should be available in the classpath.

Comments from Reviewable

ZheSun88 commented 6 years ago

Reviewed 8 of 8 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable