Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

added loadBalancerIP option to service #539

Merged
merged 1 commit into from
Mar 24, 2020
Merged

added loadBalancerIP option to service #539

merged 1 commit into from
Mar 24, 2020

Conversation

holygrolli
Copy link
Contributor

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py

opened to address #257 feature request which I need in a project. The old #323 somehow got stale.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Mar 24, 2020

💚 CLA has been signed

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

Hi @adulescentulus,
Thanks for your PR.
Can you sign the CLA: https://www.elastic.co/contributor-agreement

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

jenkins test this please

@holygrolli
Copy link
Contributor Author

@jmlrt I signed the CLA about 8 hours ago and got a pdf. I don't know what's next. Is there a manual processing step left?

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

cla/check

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

@jmlrt I signed the CLA about 8 hours ago and got a pdf. I don't know what's next. Is there a manual processing step left?

CLA check is done automatically. However it seems you signed it with your personnal email while the git commit has been done with some corporate email.

Can you change your commit email to the one you used to sign CLA?

You should be able to do that with the following commands:

git commit --amend --author="John Doe <john@doe.org>"
git push --force-with-lease

@holygrolli
Copy link
Contributor Author

@jmlrt thanks for the hint. I fixed my mail address and now CLA passes and I hope the CI as it did locally

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

jenkins test this please

@holygrolli
Copy link
Contributor Author

sorry just merged from master and so needs rebuild, didn't expect someone to be awake
But I don't understand why the build failed. Local make test works

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

jenkins test this please

@holygrolli
Copy link
Contributor Author

aaahh now I get it: between two functions there need to be two blank lines, not one like I did. I need to fix this, don't I?

@jmlrt jmlrt added the enhancement New feature or request label Mar 24, 2020
@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

aaahh now I get it: between two functions there need to be two blank lines, not one like I did. I need to fix this, don't I?

Oh yes sorry, I relaunched the test without checking logs.

Indeed our python lint test is using black and requires 2 new lines between each functions.

@holygrolli
Copy link
Contributor Author

finally hope everything is good to go now.

but why did make test did not report anything locally?

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

but why did make test did not report anything locally?

python lint job is using make lint-python

@jmlrt
Copy link
Member

jmlrt commented Mar 24, 2020

jenkins test this please

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this PR 👍

@jmlrt jmlrt merged commit 370d4a5 into elastic:master Mar 24, 2020
This was referenced Mar 24, 2020
mvk pushed a commit to ops-guru/elastic-helm-charts that referenced this pull request May 15, 2020
- based on PR: elastic#539

Signed-off-by: Max Kovgan <max@opsguru.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants