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

Removes unnecessary gradle properties #561

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

joshpalis
Copy link
Member

Description

Gradle properties seem to be directly pulled from OpenSearch gradle properties, this PR removes them as they are not necessary for this plugin. For example, systemProp.org.gradle.warning.mode=fail would fail a build outright when gradle APIs marked for future deprecation are used. This makes sense to set in OpenSearch to force a gradle version upgrade, but in this plugin's case, gradle version is only updated once OpenSearch does.

Testing locally, the build and integration test pass with these properties removed.

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.

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.62%. Comparing base (1ff9649) to head (da713fa).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #561      +/-   ##
============================================
+ Coverage     72.54%   72.62%   +0.08%     
  Complexity      665      665              
============================================
  Files            78       78              
  Lines          3416     3416              
  Branches        271      271              
============================================
+ Hits           2478     2481       +3     
+ Misses          822      818       -4     
- Partials        116      117       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

I wasted many hours figuring out that my BWCs were failing on this dang switch.

@amitgalitz
Copy link
Member

I think this is used options.forkOptions.memoryMaximumSize=2g for the integTest cluster to determine heap size, not sure how it gets default otherwise

@dbwiddis
Copy link
Member

dbwiddis commented Mar 12, 2024

I think this is used options.forkOptions.memoryMaximumSize=2g for the integTest cluster to determine heap size, not sure how it gets default otherwise

Default if not specified is 512mb.

Tests seem to be passing with this removed, though.

@amitgalitz
Copy link
Member

I think this is used options.forkOptions.memoryMaximumSize=2g for the integTest cluster to determine heap size, not sure how it gets default otherwise

Default if not specified is 512mb.

Tests seem to be passing with this removed, though.

sounds good, just checked also that :run also works

@owaiskazi19 owaiskazi19 merged commit 7a2d9d9 into opensearch-project:main Mar 12, 2024
32 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 12, 2024
Remove unecessary gradle properties

Signed-off-by: Joshua Palis <[email protected]>
(cherry picked from commit 7a2d9d9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis pushed a commit that referenced this pull request Mar 12, 2024
Removes unnecessary gradle properties (#561)

Remove unecessary gradle properties


(cherry picked from commit 7a2d9d9)

Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport PRs to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants