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.

Sign up for GitHub

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

Jump to bottom

Resolve sonar code smells #273

Merged
merged 10 commits into from
Dec 27, 2018
Merged

Resolve sonar code smells #273

merged 10 commits into from
Dec 27, 2018

Conversation

jvdb
Copy link
Contributor

@jvdb jvdb commented Dec 14, 2018 •
edited
Loading

Resolves #273.
Resolves all smells identified by Sonar.

@codecov
Copy link

codecov bot commented Dec 14, 2018 •
edited
Loading

Codecov Report

Merging #273 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #273   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity     1031   1032    +1     
=======================================
  Files            92     92           
  Lines          1393   1394    +1     
  Branches        146    146           
=======================================
+ Hits           1393   1394    +1
Impacted Files Coverage Δ Complexity Δ
.../metal/expression/value/BinaryValueExpression.java 100% <ø> (ø) 18 <0> (ø) ⬇️
.../src/main/java/io/parsingdata/metal/token/Tie.java 100% <100%> (ø) 16 <1> (ø) ⬇️
.../src/main/java/io/parsingdata/metal/token/Sub.java 100% <100%> (ø) 20 <10> (ø) ⬇️
...rc/main/java/io/parsingdata/metal/format/JPEG.java 100% <100%> (ø) 1 <0> (ø) ⬇️
...ava/io/parsingdata/metal/expression/value/Cat.java 100% <100%> (ø) 4 <2> (ø) ⬇️
...va/io/parsingdata/metal/expression/value/GUID.java 100% <100%> (ø) 8 <0> (ø) ⬇️
...ingdata/metal/expression/value/arithmetic/Sub.java 100% <100%> (ø) 2 <1> (ø) ⬇️
...singdata/metal/expression/value/reference/Ref.java 100% <100%> (ø) 17 <0> (ø) ⬇️
...ingdata/metal/expression/value/arithmetic/Mul.java 100% <100%> (ø) 2 <1> (ø) ⬇️
...ingdata/metal/expression/value/arithmetic/Div.java 100% <100%> (ø) 3 <2> (ø) ⬇️
... and 19 more

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 1ca9f74...9671985. Read the comment docs.

@parsingdata parsingdata deleted a comment from codecov bot Dec 14, 2018
@jvdb
Copy link
Contributor Author

jvdb commented Dec 14, 2018 •
edited
Loading

Removed this comment to prevent partial duplication since I've also copy/pasted it into its own issue: #274.

mvanaken
mvanaken reviewed Dec 15, 2018
View reviewed changes
Copy link
Contributor

@mvanaken mvanaken left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would have expected to als see renames of local method that clash with field names. Did you delibrately not change them? I did not read your long comment above. ;) Sorry! I agree with your proposal. :)

core/src/main/java/io/parsingdata/metal/data/ParseState.java Show resolved Hide resolved
@jvdb jvdb mentioned this pull request Dec 17, 2018
Open
@jvdb jvdb mentioned this pull request Dec 26, 2018
Closed
jvdb added 2 commits December 27, 2018 12:01
@jvdb
#272: Resolved field shadowing.
95b5066
@jvdb
#272: Replaced other concatenated strings in exception constructors w…
ace5cd4 
…ith format strings as well. Included helper method to automatically use Locale.ENGLISH.
@jvdb jvdb changed the title WIP: Resolve sonar code smells Resolve sonar code smells Dec 27, 2018
@jvdb jvdb requested a review from ccreeten December 27, 2018 11:44
@jvdb jvdb merged commit d681bad into master Dec 27, 2018
@jvdb jvdb deleted the resolve-sonar-code-smells branch December 27, 2018 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvanaken mvanaken mvanaken left review comments

@rdvdijk rdvdijk rdvdijk left review comments

@ccreeten ccreeten Awaiting requested review from ccreeten

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jvdb @rdvdijk @mvanaken