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

Add service layer #283

Merged
merged 37 commits into from Aug 13, 2020
Merged

Add service layer #283

merged 37 commits into from Aug 13, 2020

Conversation

ivanmonteiro
Copy link
Contributor

@ivanmonteiro ivanmonteiro commented Jul 3, 2020

Give the possibility to generate service layer

Fix #275

@ivanmonteiro ivanmonteiro force-pushed the add-service branch 4 times, most recently from d5c5285 to e045f61 Compare July 4, 2020 01:43
@nicolas63
Copy link
Member

Already in draft mode or you need review ?

@ivanmonteiro
Copy link
Contributor Author

Already in draft mode or you need review ?

I still have to solve some conflicts and fix. Hope to provide a review-able PR in a couple of days.

@ivanmonteiro ivanmonteiro marked this pull request as ready for review August 2, 2020 21:27
@ivanmonteiro
Copy link
Contributor Author

Already in draft mode or you need review ?

I still have to solve some conflicts and fix. Hope to provide a review-able PR in a couple of days.

I think it is ready for review.
The latest commit (service files/namespace refactoring) added lots of changes (mostly namespace changes).
If you think it is better to split this PR into smaller ones let me know.

Copy link
Member

@nicolas63 nicolas63 left a comment

Choose a reason for hiding this comment

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

With jdl we can omit service creation with service all with serviceImpl except Employee, Job
In your case the controller is good but the service is generated

@nicolas63
Copy link
Member

Maybe we can rename Infrastructure folder in WebProject to Configuration ? This can be done in other PR

@nicolas63
Copy link
Member

nicolas63 commented Aug 4, 2020

with new project there is nothing to add in dockerfile.ejs ?

@ivanmonteiro
Copy link
Contributor Author

with new project there is nothing to add in dockerfile.ejs ?

I'll check the dockerfile, it probably needs some changes

With jdl we can omit service creation with service all with serviceImpl except Employee, Job
In your case the controller is good but the service is generated

I'll check what is going on.

@nicolas63
Copy link
Member

Maybe we can rename Infrastructure folder in WebProject to Configuration ? This can be done in other PR

I will do it

@ivanmonteiro
Copy link
Contributor Author

ivanmonteiro commented Aug 4, 2020

Maybe we can rename Infrastructure folder in WebProject to Configuration ? This can be done in other PR

Agree, it is better on other PR, after this PR is merged, because some files at that folder were moved.

I will do it

Allright.

@ivanmonteiro
Copy link
Contributor Author

with new project there is nothing to add in dockerfile.ejs ?

I just changed the dockerfile, now it restores the new ..csproj projects

# Restore dependencies of .net core projects taking advantage of docker layer caching
COPY src/*/*.csproj ./src/
RUN for file in $(ls src/*.csproj); do mkdir -p ${file%.*} && mv $file ${file%.*}; done
RUN dotnet restore "src/JhipsterSampleApplication/JhipsterSampleApplication.csproj"

@ivanmonteiro
Copy link
Contributor Author

ivanmonteiro commented Aug 7, 2020

With jdl we can omit service creation with service all with serviceImpl except Employee, Job
In your case the controller is good but the service is generated

Fixed this with the latest commit

@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nicolas63
Copy link
Member

Everything looks good to me. It's a really nice job, thanks for your help on the project 😃

@nicolas63 nicolas63 merged commit abbd3b3 into jhipster:master Aug 13, 2020
@ivanmonteiro
Copy link
Contributor Author

@pascalgrimaud
Copy link
Member

@ivanmonteiro : just approved :)

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.

Add service layer
3 participants