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

Run precommit before gradle check #5066

Merged

Conversation

owaiskazi19
Copy link
Member

Signed-off-by: Owais Kazi owaiskazi19@gmail.com

Description

This PR adds dependency of precommit to run successfully before running gradle check. Thus saving time and collecting errors before waiting for the entire gradle check suite to run.

Dependency chart: spotless -> precommit -> gradle check

For more information: #5027 and #4679

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@owaiskazi19 owaiskazi19 requested a review from dbwiddis November 3, 2022 22:41
@owaiskazi19 owaiskazi19 requested review from a team and reta as code owners November 3, 2022 22:41
@owaiskazi19 owaiskazi19 changed the title Run spotless before gradle check Run precommit before gradle check Nov 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Gradle Check (Jenkins) Run Completed with:

uses: actions/setup-java@v2
with:
java-version: 11
distribution: adopt
Copy link
Collaborator

Choose a reason for hiding this comment

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

distribution: temurin please

Copy link
Member Author

Choose a reason for hiding this comment

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

I copy/pasted precommit from main which has adopt distribution. Updated it.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2022

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #5066 (1bd2feb) into main (fab4336) will decrease coverage by 0.00%.
The diff coverage is 61.93%.

@@             Coverage Diff              @@
##               main    #5066      +/-   ##
============================================
- Coverage     70.84%   70.83%   -0.01%     
- Complexity    57867    58076     +209     
============================================
  Files          4692     4704      +12     
  Lines        276650   277309     +659     
  Branches      40155    40170      +15     
============================================
+ Hits         196002   196444     +442     
- Misses        64500    64765     +265     
+ Partials      16148    16100      -48     
Impacted Files Coverage Δ
...java/org/opensearch/upgrade/ValidateInputTask.java 84.61% <0.00%> (ø)
...ch/analysis/common/CommonAnalysisModulePlugin.java 92.33% <0.00%> (+0.05%) ⬆️
.../src/main/java/org/opensearch/LegacyESVersion.java 59.34% <ø> (-2.37%) ⬇️
...ecommission/awareness/put/DecommissionRequest.java 85.71% <0.00%> (-6.60%) ⬇️
...min/cluster/stats/TransportClusterStatsAction.java 70.83% <ø> (ø)
...ensearch/action/search/SearchTransportService.java 86.80% <ø> (-0.20%) ⬇️
...va/org/opensearch/action/update/UpdateRequest.java 47.35% <0.00%> (+2.90%) ⬆️
...arch/cluster/decommission/DecommissionService.java 35.65% <0.00%> (+0.38%) ⬆️
.../cluster/metadata/IndexNameExpressionResolver.java 83.79% <ø> (-0.03%) ⬇️
...va/org/opensearch/cluster/node/DiscoveryNodes.java 89.72% <ø> (-0.18%) ⬇️
... and 592 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Change the JDK distribution to keep it consistent.

Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19 owaiskazi19 force-pushed the run-spotless-before-gradle-check branch from f29ae69 to 0e309b5 Compare November 7, 2022 22:46
@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Nov 7, 2022

Github Action worked fine on my fork.
The sequence was spotless -> precommit -> gradle check. Not sure why it's not working the same way here.
cc: @reta @peterzhuamazon

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19 owaiskazi19 requested review from dblock and reta November 8, 2022 01:15
@reta
Copy link
Collaborator

reta commented Nov 8, 2022

Github Action worked fine on my fork. The sequence was spotless -> precommit -> gradle check. Not sure why it's not working the same way here. cc: @reta @peterzhuamazon

Hm .. it seems like the Github actions are being taken from main branch, may be because the branch is protected? (and it is not in your fork)

@dblock dblock merged commit 2139e80 into opensearch-project:main Nov 8, 2022
@peterzhuamazon
Copy link
Member

peterzhuamazon commented Nov 8, 2022

This PR has caused a lot of issue and is not even checking out the correct commit to run spotless and gradle precommit:
See:

mch2 added a commit that referenced this pull request Nov 8, 2022
mch2 added a commit to mch2/OpenSearch that referenced this pull request Nov 8, 2022
This reverts commit 2139e80.

Signed-off-by: Marc Handalian <handalm@amazon.com>
dblock pushed a commit that referenced this pull request Nov 8, 2022
This reverts commit 2139e80.

Signed-off-by: Marc Handalian <handalm@amazon.com>

Signed-off-by: Marc Handalian <handalm@amazon.com>
@owaiskazi19
Copy link
Member Author

This PR has caused a lot of issue and is not even checking out the correct commit to run spotless and gradle precommit: See:

We were under the impression it's because of the branch protection rule. What are the correct commits to run spotless and precommit @peterzhuamazon ?

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.

5 participants