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

ORC-1487: Enable checkstyle on src/test with checkstyle-suppressions.xml #1591

Closed
wants to merge 2 commits into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Aug 21, 2023

What changes were proposed in this pull request?

This PR aims to enable checkstyle on src/test directories to validate the recent PRs like #1590 .

Why are the changes needed?

To help the recent community work, we permanently declare the following rules as exception in test source code.

     <suppress checks="LineLength" files="src/test/*"/>
     <suppress checks="NewlineAtEndOfFile" files="src/test/*"/>

     <suppress checks="UnusedImports" files="src/test/*"/>
     <suppress checks="AvoidStarImport" files="src/test/*"/>
     <suppress checks="CustomImportOrder" files="src/test/*"/>

The other suppressed rules will be removed after we finish the test code clean-ups.

How was this patch tested?

Pass the CIs.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Aug 21, 2023

cc @mystic-lama , @williamhyun

@paliwalashish
Copy link
Contributor

@dongjoon-hyun Awesome. I looked at adding this but was not sure if it was good idea. Thank you so much for this.

Do we want to make Line length as change as part of this PR?

Copy link
Contributor

@paliwalashish paliwalashish left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@dongjoon-hyun
Copy link
Member Author

Thank you, @mystic-lama . During reviewing your on-going PR, I evaluated our remaining work in the existing code and decided to ignore LineLength and NewlineAtEndOfFile and three Import-related completely in 2.0.0. It will reduce the number of changed line in your PR and future PRs on benchmark module.

@dongjoon-hyun dongjoon-hyun added this to the 2.0.0 milestone Aug 21, 2023
@paliwalashish
Copy link
Contributor

Thanks a ton @dongjoon-hyun
We can address them later

@dongjoon-hyun
Copy link
Member Author

Yes, we can handle them gradually in 2.1.0 or later.

@dongjoon-hyun dongjoon-hyun deleted the ORC-1487 branch August 21, 2023 16:44
dongjoon-hyun added a commit that referenced this pull request Aug 21, 2023
…ions.xml`

### What changes were proposed in this pull request?

This PR aims to enable `checkstyle` on `src/test` directories to validate the recent PRs like #1590 .

### Why are the changes needed?

To help the recent community work, we permanently declare the following rules as exception in test source code.

```xml
     <suppress checks="LineLength" files="src/test/*"/>
     <suppress checks="NewlineAtEndOfFile" files="src/test/*"/>

     <suppress checks="UnusedImports" files="src/test/*"/>
     <suppress checks="AvoidStarImport" files="src/test/*"/>
     <suppress checks="CustomImportOrder" files="src/test/*"/>
```

The other suppressed rules will be removed after we finish the test code clean-ups.

### How was this patch tested?

Pass the CIs.

Closes #1591 from dongjoon-hyun/ORC-1487.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit d3d3b60)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun dongjoon-hyun modified the milestones: 2.0.0, 1.9.2 Aug 21, 2023
@dongjoon-hyun
Copy link
Member Author

I backported this test infra PR to branch-1.9 and adjusted the JIRA issue versions.

cxzl25 pushed a commit to cxzl25/orc that referenced this pull request Jan 11, 2024
…ions.xml`

### What changes were proposed in this pull request?

This PR aims to enable `checkstyle` on `src/test` directories to validate the recent PRs like apache#1590 .

### Why are the changes needed?

To help the recent community work, we permanently declare the following rules as exception in test source code.

```xml
     <suppress checks="LineLength" files="src/test/*"/>
     <suppress checks="NewlineAtEndOfFile" files="src/test/*"/>

     <suppress checks="UnusedImports" files="src/test/*"/>
     <suppress checks="AvoidStarImport" files="src/test/*"/>
     <suppress checks="CustomImportOrder" files="src/test/*"/>
```

The other suppressed rules will be removed after we finish the test code clean-ups.

### How was this patch tested?

Pass the CIs.

Closes apache#1591 from dongjoon-hyun/ORC-1487.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants