Skip to content

Conversation

@tvernum
Copy link
Contributor

@tvernum tvernum commented Jul 19, 2023

If the system properties and jvm args are both non-empty then we need a space between them.

If the system properties and jvm args are both non-empty then we need
a space between them.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

+ " "
+ featureFlagProperties
+ systemProperties
+ ' '
Copy link
Contributor

Choose a reason for hiding this comment

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

Good sleuthing! Do we have the same potential issue between all these string values? Maybe we should build this string in a similar way to previous ones, with a stream and joiner.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, a stream and joiner would seem more robust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry about that. It was the end of the day and I wanted to get something out the door, so I ignored the screaming voice inside me telling me how dirty the fix was.
🙀
Updated in d6ea5b3

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.

LGTM

@tvernum tvernum merged commit 58906f9 into elastic:main Jul 25, 2023
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 >non-issue Team:Delivery Meta label for Delivery team v8.9.1 v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants