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

disbaled javadoc check #392

Merged
merged 4 commits into from Mar 7, 2019

Conversation

@pbrahmbhatt3
Copy link
Contributor

commented Mar 6, 2019

Disbaled the checks for javadoc from checklist.xml by assigning "ignore" to value property.

Priyam Brahmbhatt
@pbrahmbhatt3

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

@wslulciuc I am having difficulty figuring out why this fails. Any suggestions

@@ -231,21 +231,21 @@
value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/>
</module>
<module name="JavadocMethod">
<property name="scope" value="public"/>

This comment has been minimized.

Copy link
@wslulciuc

wslulciuc Mar 7, 2019

Member

I believe we only need to change the checkstyle scope to ignore JavaDoc on methods. Can you try changing the scope from
public to nothing?

 <property name="scope" value="nothing"/>

This comment has been minimized.

Copy link
@pbrahmbhatt3

pbrahmbhatt3 Mar 7, 2019

Author Contributor

sure. Thanks. :D Can i delete this PR and make a new one?

This comment has been minimized.

Copy link
@wslulciuc

wslulciuc Mar 7, 2019

Member

I'd be easier (I think) if you reverted the values you introduced and only modify the scope?

Copy link
Member

left a comment

@pbrahmbhatt3: The CI build is failing because invalid values are being provided for properties associated with JavadocMethod (see: http://checkstyle.sourceforge.net/config_javadoc.html#JavadocMethod)

Once you address a minor comment I left, you'll be able to merge this change!

@codecov

This comment has been minimized.

Copy link

commented Mar 7, 2019

Codecov Report

Merging #392 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #392   +/-   ##
=========================================
  Coverage     76.61%   76.61%           
  Complexity      394      394           
=========================================
  Files            90       90           
  Lines          1022     1022           
  Branches         48       48           
=========================================
  Hits            783      783           
  Misses          172      172           
  Partials         67       67

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 41cc011...a494e57. Read the comment docs.

@@ -6,12 +6,9 @@
<!--
Checkstyle configuration that checks the Google coding conventions from Google Java Style
that can be found at https://google.github.io/styleguide/javaguide.html.

This comment has been minimized.

Copy link
@wslulciuc

wslulciuc Mar 7, 2019

Member

Minor: The newlines here help with readability. Mind adding them back?

Priyam Brahmbhatt
Copy link
Member

left a comment

Thanks, @pbrahmbhatt3! This will dramatically reduce the logged warnings around checkstyle in our builds.

@wslulciuc wslulciuc merged commit d0bc9ec into MarquezProject:master Mar 7, 2019
4 checks passed
4 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 41cc011...a494e57
Details
codecov/project 76.61% remains the same compared to 41cc011
Details
@pbrahmbhatt3

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

Thanks for your help! You are the best. You have been really helpful. @wslulciuc

@wslulciuc wslulciuc referenced this pull request May 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.