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

Convert Version to Java - clusterformation part1 #32009

Merged
merged 3 commits into from
Jul 13, 2018

Conversation

alpar-t
Copy link
Contributor

@alpar-t alpar-t commented Jul 12, 2018

  • This allows to move all all .java files from the groovy source set
    • @colings86 , this will prevent eclipse from tangling up in this setup. I tested it and eclipse shows no problems now.
  • make it possible to use Version from clusterformation Java code

There is no change in functionality.
There are also some additional tests for Version.

- This allows to move all  all .java files from .groovy.
- Will prevent eclipse from tangling up in this setup
- make it possible to use Version from Java
@alpar-t alpar-t added the :Delivery/Build Build or test infrastructure label Jul 12, 2018
@alpar-t alpar-t requested review from nik9000 and rjernst July 12, 2018 14:31
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@colings86
Copy link
Contributor

@colings86 , this will prevent eclipse from tangling up in this setup. I tested it and eclipse shows no problems now

Awesome, thanks so much for doing this ❤️

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This looks good. I left some minor requests, if you agree no further need for review.

return Objects.hash(major, minor, revision, id, snapshot, suffix);
}

public final int getMajor() {
Copy link
Member

Choose a reason for hiding this comment

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

I would make the class final, and these members public final

return snapshot;
}

public final boolean isSnapshot() {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you have get and is? If we don't make these public final members, then at least there should only be one of these methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just an oversight. I'm removing get

@@ -69,6 +69,10 @@ public static void main(String[] args) throws IOException {
fail("unsupported argument '" + arg + "'");
}
}
if (rootPathList == null) {
fail("No paths provided");
return;
Copy link
Member

Choose a reason for hiding this comment

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

There should be no need for a return here since fail throws an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It calls System.exit which is not picked up and IDE complains below that rootPathList might be null.

@@ -223,7 +223,8 @@ class VersionCollectionTests extends GradleUnitTestCase {
Version.fromString("5.1.1"), Version.fromString("5.2.0"), Version.fromString("5.2.1"),
Version.fromString("5.3.0"), Version.fromString("5.3.1")]

assertTrue(wireCompatList.containsAll(vc.wireCompatible))
def compatible = vc.wireCompatible
Copy link
Member

Choose a reason for hiding this comment

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

Please use a concrete type, eg List

@alpar-t alpar-t merged commit 23198ea into elastic:master Jul 13, 2018
@alpar-t alpar-t deleted the fix/30904-cluster-formation-part1 branch July 13, 2018 08:35
alpar-t added a commit that referenced this pull request Jul 18, 2018
Implement buildSrc Version in java

- This allows to move all  all .java files from .groovy.
- Will prevent eclipse from tangling up in this setup
- make it possible to use Version from Java

This backport pulls in some changes from the non backported #27397
but that should be fine
dnhatn added a commit that referenced this pull request Jul 19, 2018
* 6.x:
  Fix rollup on date fields that don't support epoch_millis (#31890)
  Revert "Introduce a Hashing Processor (#31087)" (#32179)
  [test] use randomized runner in packaging tests (#32109)
  Painless: Fix caching bug and clean up addPainlessClass. (#32142)
  Fix BwC Tests looking for UUID Pre 6.4 (#32158) (#32169)
  Call setReferences() on custom referring tokenfilters in _analyze (#32157)
  Add more contexts to painless execute api (#30511)
  Add EC2 credential test for repository-s3 (#31918)
  Fix CP for namingConventions when gradle home has spaces (#31914)
  Convert Version to Java - clusterformation part1 (#32009)
  Fix Java 11 javadoc compile problem
  Improve docs for search preferences (#32098)
  Configurable password hashing algorithm/cost(#31234) (#32092)
  [DOCS] Update TLS on Docker for 6.3
  ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are
  Switch distribution to new style Requests (#30595)
  Build: Skip jar tests if jar disabled
  Build: Move shadow customizations into common code (#32014)
  Painless: Add PainlessClassBuilder (#32141)
  Fix accidental duplication of bwc test for script behavior
  Handle missing values in painless (#30975) (#31903)
  Build: Make additional test deps of check (#32015)
  Painless: Fix Bug with Duplicate PainlessClasses (#32110)
  Adjust translog after versionType removed in 7.0 (#32020)
  Disable C2 from using AVX-512 on JDK 10 (#32138)
  [Rollup] Add new capabilities endpoint for concrete rollup indices (#32111)
  Mute :qa:mixed-cluster indices.stats/10_index/Index - all’
  [ML] Wait for aliases in multi-node tests (#32086)
  Ensure to release translog snapshot in primary-replica resync (#32045)
  Docs: Fix missing example script quote (#32010)
  Add Index UUID to `/_stats` Response (#31871) (#32113)
  [ML] Move analyzer dependencies out of categorization config (#32123)
  [ML][DOCS] Add missing 6.3.0 release notes (#32099)
  Updates the build to gradle 4.9 (#32087)
  Update monitoring template version to 6040099 (#32088)
  Fix put mappings java API documentation (#31955)
  Add exclusion option to `keep_types` token filter (#32012)
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants