-
Notifications
You must be signed in to change notification settings - Fork 249
Handful of fixes, mostly related to project compilation. #267
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
Conversation
- Also fixes IntelliJ IDEA not properly marking the folder as Generated Sources
…but not on their GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much.
Generally we have to look about the UT-TACO-TOKEN.
In terms of concept and naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes are very good. Thank you
@@ -49,6 +49,11 @@ public class GoogleAuthenticateTest { | |||
private AuthenticationService authenticationService; | |||
|
|||
@Test | |||
public void testGoogleRegisterSuccess() { | |||
assertTrue(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally I would not use a test like this because it gives false hope that the functionality works.
Better approach is to keep the test as is and annotate it with @disabled with a brief description:
@Disabled("Disabled until properly refactored")
@@ -301,7 +303,12 @@ public void testCreateApplicationSuccess() { | |||
.verifyComplete(); | |||
} | |||
|
|||
@SuppressWarnings("ConstantConditions") | |||
@Test | |||
public void testUpdateApplicationFailedDueToLackOfDatasourcePermissions() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to keep the original test and annotate it with @disabled annotation with a brief description. Otherwise it gives false hope that the functionality works.
@Disabled("Disabled until properly refactored")
Thank you very much for your contribution! We took most of your suggestions and realized it in other branches in the meantime. Happily, we can close now this PR. |
This branch contains a handful of fixes, you can cherry pick whichever you like: