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

made tests use a unified test JCM - bad indentations #78

Merged
merged 9 commits into from
May 25, 2020

Conversation

guiguilechat
Copy link

@guiguilechat guiguilechat commented Jan 20, 2020

also reduces the overload of constructor.
see #74

@codecov-io
Copy link

codecov-io commented Jan 20, 2020

Codecov Report

Merging #78 into master will increase coverage by 0.3%.
The diff coverage is 59.18%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master      #78     +/-   ##
===========================================
+ Coverage     38.91%   39.21%   +0.3%     
- Complexity      996     1010     +14     
===========================================
  Files           132      134      +2     
  Lines          5636     5669     +33     
  Branches        840      847      +7     
===========================================
+ Hits           2193     2223     +30     
- Misses         3156     3159      +3     
  Partials        287      287
Impacted Files Coverage Δ Complexity Δ
...el/exceptions/JCaseSensitivityChangeException.java 100% <100%> (ø) 1 <1> (?)
.../main/java/com/helger/jcodemodel/JResourceDir.java 80.7% <20%> (-2.94%) 28 <1> (ø)
...main/java/com/helger/jcodemodel/AbstractJType.java 50% <25%> (ø) 22 <0> (ø) ⬇️
...odemodel/exceptions/JInvalidFileNameException.java 40% <40%> (ø) 1 <1> (?)
...rc/main/java/com/helger/jcodemodel/JCodeModel.java 65.84% <75%> (+3.79%) 51 <2> (+10) ⬆️
...c/main/java/com/helger/jcodemodel/util/FSName.java 80.95% <0%> (+4.76%) 8% <0%> (+1%) ⬆️
...ava/com/helger/jcodemodel/JCodeModelException.java 60% <0%> (+40%) 2% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccb5a0e...857c053. Read the comment docs.

@phax
Copy link
Owner

phax commented Jan 21, 2020

That is a huge change and I partially dislike it. It also breaks backwards compatibility.
First of all, please remove all the superflowous brackets - I hate them - thx.

Also please remove all the serialVersionUID constants - that's bad style that they have the value 1 and they are created by the runtime automatically.

No public members in classes please.

For the other topics from #74 I will respond separately, but I don't have time for this today

@guiguilechat
Copy link
Author

guiguilechat commented Jan 21, 2020

Why exactly would it break backward compatibility ? I did not know following commits would be added to the PR

I don't know what are superflowous brackets ?
I have my coding style implemented in eclipse, so whenever I save a file it forces the code style on the lines I saved. Do you have an auto format script so that my java files are automatically formatted to your own style when I push on git ? I already found one for the tab/spaces, seems like something possible.

I added the serialversion id because eclipse complains about them. I will add @ignorewarning to them instead (in which classes ?)

I put members public in the exceptions because they are final and unmodifiable. But of course putting them as getX() is just as good.

@stale
Copy link

stale bot commented Apr 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 22, 2020
@phax phax added pinned and removed wontfix labels Apr 23, 2020
@phax phax self-assigned this Apr 23, 2020
@phax phax merged commit ec12c7f into phax:master May 25, 2020
@guiguilechat guiguilechat deleted the API_guiguigui branch May 25, 2020 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants