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

addressing build issue with s3-sink #2757

Closed

Conversation

cmanning09
Copy link
Contributor

@cmanning09 cmanning09 commented May 25, 2023

Description

Addresses build failure observed in PR: #2726

* What went wrong:
Execution failed for task ':data-prepper-plugins:s3-sink:spotlessMarkdownCheck'.
> The following files had format violations:
      README.md
          @@ -72,4 +72,3 @@
           ```
           ./gradlew·:data-prepper-plugins:s3-sink:integrationTest·-Dtests.s3sink.region=<your-aws-region>·-Dtests.s3sink.bucket=<your-bucket>
           ```
          -
  Run './gradlew :data-prepper-plugins:s3-sink:spotlessApply' to fix these violations.

See: https://github.com/opensearch-project/data-prepper/actions/runs/5082697390/jobs/9132752111?pr=2726

Issues Resolved

None, pro-active clean up

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

Signed-off-by: Christopher Manning <cmanning09@users.noreply.github.com>
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Merging #2757 (d1cba67) into main (3ce5b53) will increase coverage by 0.19%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2757      +/-   ##
============================================
+ Coverage     93.81%   94.00%   +0.19%     
- Complexity     2315     2354      +39     
============================================
  Files           269      276       +7     
  Lines          6482     6573      +91     
  Branches        534      540       +6     
============================================
+ Hits           6081     6179      +98     
+ Misses          265      254      -11     
- Partials        136      140       +4     

see 8 files with indirect coverage changes

Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

I think I addressed this in #2755, but as this has no conflicts it should be ok.

@dlvenable
Copy link
Member

@cmanning09 , My main is passing. Maybe we should leave this be?

@cmanning09
Copy link
Contributor Author

@cmanning09 , My main is passing. Maybe we should leave this be?

@dlvenable , great. Thanks for addressing this. I will close this. At a high level are we not leveraging our approval workflows correctly? Ideally, this shouldn't have been committed and impacted other PRs.

@cmanning09 cmanning09 closed this May 25, 2023
@dlvenable
Copy link
Member

@cmanning09 , I agree with that. We shouldn't have let that get through.

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.

None yet

2 participants