jakartaee / enterprise-beans

Jakarta Enterprise Beans
https://eclipse.org/ee4j/ejb
Other
19 stars 29 forks source link

Jakarta 9 wave0 - project structure and formatting #118

Closed jeanouii closed 4 years ago

jeanouii commented 4 years ago

Trying to get this first PR as there isn't much in there but Maven tweaks. I basically used the configuration of the Jakarta Servlet project so Jakarta Enterprise Beans is consistent and uses the same rules.

I ran the build to reformat and fix everything based on the copyright/formatting rules set.

So there are a lot of changes but in the end, most of them are formatting after running the full build.

Hope it helps.

anthonyvdotbe commented 4 years ago

Just a quick comment: commit messages like Comment from ederks85. Thanks! are not helpful when looking through the git history. I would just squash the last 3 commits into one and write something like Correct capitalization of Enterprise Beans instead.

jeanouii commented 4 years ago

Thanks @anthonyvdotbe for the feedback.

hussainnm commented 4 years ago

@jeanouii The addition of format validation plugins is fine. The changes made to felix, jar and javadoc plugins are incorrect. The removal of spec-version-maven-plugin is incorrect. The addition of glassfish-copyright is fine but the copyright.txt included adds a duplicate All rights reserved line to all files when applied.

My suggestion is to review #109 and merge it to master and then re-build your changes on top of that.

jeanouii commented 4 years ago

@hussainnm Sorry was out for 2 weeks on vacation. Back to business, I reviewed today your PR and merged it. Can you rebase your PR.

This one, I'm not sure what's left compared to your PR. I need to spend some time. But probably easier to close it and start the review process again on a brand new PR. What do you think?

hussainnm commented 4 years ago

Yes, it would be better to close this PR and create a new one with the formatting and copyright plugin changes. Also I noticed the jakarta-wave0 and jakarta-wave0-javax branches are pushed directly in this repo. If possible can you delete these branches in this repository.

jeanouii commented 4 years ago

No point in leaving this one opened now. I'll create a smaller one but focused PR