-
Notifications
You must be signed in to change notification settings - Fork 0
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
Tickets #43
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #43 +/- ##
=============================================
+ Coverage 60.81% 79.18% +18.37%
- Complexity 59 104 +45
=============================================
Files 24 34 +10
Lines 245 370 +125
Branches 7 7
=============================================
+ Hits 149 293 +144
+ Misses 92 73 -19
Partials 4 4 ☔ View full report in Codecov by Sentry. |
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.
@hizmailovich check my comments, please
src/main/java/git/tracehub/pmo/controller/ProjectController.java
Outdated
Show resolved
Hide resolved
src/main/java/git/tracehub/pmo/controller/TicketController.java
Outdated
Show resolved
Hide resolved
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.
@hizmailovich regarding table 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.
@hizmailovich take a look at my comments, please
src/main/java/git/tracehub/pmo/controller/ProjectController.java
Outdated
Show resolved
Hide resolved
* | ||
* @since 0.0.0 | ||
*/ | ||
public interface Platform { |
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.
the design of this interface is not scalable, since you will add methods when something will be changed. All your methods are operating with 2 params: Scalar<String> token
and Scalar<String> location
, so first thing you can do is to introduce just one method that does all of that above:
final class Some implements Platform {
private final Scalar<String> token;
private final Scalar<String> location;
void prepare() {
// prepare some platform since we have location and token
}
}
also, I suggest to encapsulate not a location
and token
but a whole platform
itself, like Github
-> RtGithub
. I think we can something similar with GitLab, Bitbucket and so on
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.
I used that approach to create a map of beans (platforms) to avoid if-else, but your idea looks better.
Regarding platform encapsulation, it's not suitable here, because of CreateWebhook
action, which uses http-requests instead of RtGithub
, thus we have to encapsulate both location
and token
@@ -153,4 +201,17 @@ void throwsOnCreatingInvalidProject() throws SQLException { | |||
).affirm(); | |||
} | |||
|
|||
private void mock(final Project project) throws SQLException { |
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.
how about introducing fake project?
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.
I don't need to retrieve a project as a result, I just want to inject this project into result set
But I removed this private method
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.
@hizmailovich just one comment
* | ||
* @return Map of implemented platforms | ||
*/ | ||
@Bean |
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.
can we automate this one using collection autowiring in spring?
@rultor merge |
@h1alexbel Thanks for your request; @hizmailovich please confirm this. |
@rultor merge |
@h1alexbel take a look, please
Closes #34
Closes #11
Closes #47
PR-Codex overview
This PR focuses on renaming tables and columns in the database schema. It also includes changes related to projects, tickets, and requests.
Detailed summary
projects.projects
table toprojects.project
projects.performers
table toprojects.performer
Projects.byUser
method parameter fromemail
tologin
Projects.employ
method withScalar<Project>
parameterProjects.byId
method withUUID
parameter