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

style: update checkstyle + turn on some rules #847

Merged
merged 5 commits into from Jul 23, 2017

Conversation

Projects
None yet
6 participants
@AlexElin
Contributor

AlexElin commented Jun 23, 2017

  1. Update checkstyle to 7.8.2
  2. Add checkstyle module "RedundantModifier"
  3. Uncomment checkstyle modules "ArrayTypeStyle", "ModifierOrder"
style: update checkstyle + turn on some rules
1. Update checkstyle to 7.8.2
2. Add checkstyle module "RedundantModifier"
3. Uncomment checkstyle modules "ArrayTypeStyle", "ModifierOrder"
@codecov-io

This comment has been minimized.

codecov-io commented Jun 23, 2017

Codecov Report

Merging #847 into master will decrease coverage by <.01%.
The diff coverage is 79.13%.

@@             Coverage Diff              @@
##             master     #847      +/-   ##
============================================
- Coverage     65.25%   65.25%   -0.01%     
+ Complexity     3519     3518       -1     
============================================
  Files           166      166              
  Lines         15253    15253              
  Branches       2474     2474              
============================================
- Hits           9954     9953       -1     
  Misses         4103     4103              
- Partials       1196     1197       +1
@davecramer

This comment has been minimized.

Member

davecramer commented Jun 24, 2017

I like the idea of checkstyle but this seems like a pretty invasive change for relatively little benefit?

What is the new rule supposed to prevent/fix/help ?

@jorsol

This comment has been minimized.

Contributor

jorsol commented Jun 24, 2017

It's a pretty invasive change, but I think that it help to have more quality in the code, force the uniform style in general is a good practice.

Merge branch 'master' into checkstyle
# Conflicts:
#	pgjdbc/src/main/java/org/postgresql/util/PSQLState.java
@AlexElin

This comment has been minimized.

Contributor

AlexElin commented Jun 26, 2017

@davecramer the new rule suppused to force uniform style, prevent from writing a needless code (e.g. public modifier in inerface's methods, nested enums as static and so on)

@panchenko

This comment has been minimized.

panchenko commented Jun 26, 2017

Most changes seems to be array declarations, which matches the Google code style documentation 4.8.3.2 No C-style array declarations

@vlsi

This comment has been minimized.

Member

vlsi commented Jun 26, 2017

I'm inclined to +1 the change

@davecramer

This comment has been minimized.

Member

davecramer commented Jun 26, 2017

@vlsi vlsi merged commit 246b759 into pgjdbc:master Jul 23, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@vlsi vlsi added this to the 42.1.4 milestone Jul 23, 2017

@AlexElin AlexElin deleted the AlexElin:checkstyle branch Aug 1, 2017

davecramer added a commit to davecramer/pgjdbc that referenced this pull request Sep 19, 2017

style: update checkstyle + turn on some rules (pgjdbc#847)
1. Update checkstyle to 7.8.2
2. Add checkstyle module "RedundantModifier"
3. Uncomment checkstyle modules "ArrayTypeStyle", "ModifierOrder"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment