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

refactor: renamed method to avoid checkstyle error #3652

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

purplefox
Copy link
Contributor

Description

Rename a method that triggers a checkstyle error.

Testing done

N/A

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@purplefox purplefox requested a review from a team as a code owner October 23, 2019 00:59
@vcrfxia vcrfxia changed the title fix: renamed method to avoid checkstyle error refactor: renamed method to avoid checkstyle error Oct 23, 2019
@vcrfxia
Copy link
Contributor

vcrfxia commented Oct 23, 2019

Did we change checkstyle configurations recently? This method appears to have been introduced two months ago, and checkstyle errors fail our builds.

(Also changed the PR title from "fix" to "refactor" as "fix" is for bugfixes and appear in the auto-generated changelog while other types of commits such as "refactor" and "style" do not. Perhaps the label for this change should be "style" rather than "refactor" -- I don't feel strongly.)

@purplefox
Copy link
Contributor Author

Did we change checkstyle configurations recently? This method appears to have been introduced two months ago, and checkstyle errors fail our builds.

(Also changed the PR title from "fix" to "refactor" as "fix" is for bugfixes and appear in the auto-generated changelog while other types of commits such as "refactor" and "style" do not. Perhaps the label for this change should be "style" rather than "refactor" -- I don't feel strongly.)

I'm not sure if there were changes recently, but I always notice this error if I run checks on all files in the repository, and it's been annoying me ;)

@purplefox
Copy link
Contributor Author

I'm really confused about how the checkstyle maven config works. According to https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/custom-checker-config.html the default settings is sun_checks.xml if none is specified, and we don't seem to specify any in the config. However if I explicit configure it to use sun_checks.xml I get different checking behaviour. Go figure! ;)

@agavra
Copy link
Contributor

agavra commented Oct 23, 2019

@purplefox - I think we inherit the checkstyle from a parent pom (look at common-parent.pom). Im trying to track down exactly where the resource is.

some context: #1589

Copy link
Contributor

@agavra agavra left a comment

Choose a reason for hiding this comment

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

Anyway... no reason holding this up

@agavra agavra requested a review from a team October 23, 2019 23:08
@purplefox purplefox merged commit a8a3588 into confluentinc:master Oct 23, 2019
@purplefox purplefox deleted the checkstyle_err branch October 28, 2019 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants