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

Add checkstyle for non-inclusive terms #1782

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

peternied
Copy link
Member

Description

To aid in detection and removal of non-inclusive terms, adding a
checkstyle error when they are detected. When we are ready to remove
all these terms from the codebase we will set these checks to error
level.

Issues Resolved

Updated build output

Since all errors are ignored by default, now the checkstyle output is much easier to parse, here is an example of the output with this change.

...
[ant:checkstyle] [WARN] /local/home/petern/git/security/src/main/java/org/opensearch/security/tools/SecurityAdmin.java:1267: Usage should be switched to an allow* based pattern [RegexpSingleline]
[ant:checkstyle] [WARN] /local/home/petern/git/security/src/main/java/org/opensearch/security/tools/SecurityAdmin.java:1268: Usage should be switched to an allow* based pattern [RegexpSingleline]
[ant:checkstyle] [WARN] /local/home/petern/git/security/src/main/java/org/opensearch/security/tools/SecurityAdmin.java:1278: Usage should be switched to an allow* based pattern [RegexpSingleline]
Checkstyle rule violations were found. See the report at: file:///local/home/petern/git/security/build/reports/checkstyle/main.html
Checkstyle files with violations: 22
Checkstyle violations by severity: [warning:123]
...

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

To aid in detection and removal of non-inclusive terms, adding a
checkstyle error when they are detected.  When we are ready to remove
all these terms from the codebase we will set these checks to error
level.

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied peternied added the enhancement New feature or request label Apr 21, 2022
@peternied peternied requested a review from a team April 21, 2022 17:53
@davidlago
Copy link

Let's bubble this up to project level, this is a great idea @peternied !

@peternied
Copy link
Member Author

Let's bubble this up to project level, this is a great idea @peternied !

@davidlago Done!

@.tlfeng Thanks for driving this effort, I would strongly recommend adding a mechanism to ensure we have removed all of these terms. By onboarding a separate check style workflow that can be created and run automatically by plugins it ensure compliance and lets you capture metrics charting the progress to zero.

I've created a couple of checkstyle rules in this PR for the security plugin #1782

From opensearch-project/OpenSearch#472 (comment)

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

Awesome!

@peternied peternied changed the title Add checkstyle errors for non-inclusive terms Add checkstyle for non-inclusive terms Apr 21, 2022
@peternied peternied merged commit 8e3d781 into opensearch-project:main Apr 21, 2022
@peternied peternied deleted the inclusive-name-checks branch April 21, 2022 18:38
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants