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

573 allow to add remove elements from catalog dynamically through api #574

Conversation

apupier
Copy link
Contributor

@apupier apupier commented Oct 12, 2020

No description provided.

this is a best practice recommended by Sonar. The public modifier was
mandatory with Junit4, it is no more the case with JUnit 5.

Signed-off-by: Aurélien Pupier <apupier@redhat.com>
- removed getConnectorAsJson from public API . I'm not sure that it is
very useful now that we can have the Java model and add a connector. it
is also simplifying the API to add a connector

Signed-off-by: Aurélien Pupier <apupier@redhat.com>
Signed-off-by: Aurélien Pupier <apupier@redhat.com>
@apupier apupier force-pushed the 573-AllowToAddremoveElementsFromCatalogDynamicallyThroughAPI branch from 48f86a8 to bb5437c Compare October 12, 2020 13:17
Signed-off-by: Aurélien Pupier <apupier@redhat.com>
@apupier apupier force-pushed the 573-AllowToAddremoveElementsFromCatalogDynamicallyThroughAPI branch from bb5437c to 2b2123f Compare October 12, 2020 13:30
Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

Just two things: the method name is not very important. The import could be easier and just use the needed classes.

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't import with * operator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was only using the defaults of Eclipse projects but fine to follow another rule.

even for JUnit assertions

Signed-off-by: Aurélien Pupier <apupier@redhat.com>
@oscerd
Copy link
Contributor

oscerd commented Oct 12, 2020

LGTM. Let me merge.

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