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

cleanup: unused code and pipelines #322

Merged
merged 4 commits into from
Oct 15, 2019
Merged

Conversation

alexeldeib
Copy link
Contributor

What this PR does / why we need it:
cleanup duplicate iam code and unused clients, wait more efficiently for cert-manager pods during testing

Special notes for your reviewer:

If applicable:

  • this PR contains documentation
  • this PR contains tests

@alexeldeib
Copy link
Contributor Author

this doesn't contain any functional changes -- not sure why it fails CI? same test as the other PR, eventhub consumergroup? "works on my machine" running e2e 🙁

@szoio
Copy link
Contributor

szoio commented Oct 13, 2019

this doesn't contain any functional changes -- not sure why it fails CI? same test as the other PR, eventhub consumergroup? "works on my machine" running e2e 🙁

Hi @alexeldeib - I had the same problem. I think the problem is the eventhub is fails to create in the test suite setup, which causes the consumer group to fail to create. I've fixed this in #348. Btw I really like the approach you took on Incendiary-iguana to abstract out the common controller code.

@alexeldeib
Copy link
Contributor Author

alexeldeib commented Oct 14, 2019

Thanks @szoio! Looks like I need to rebase both, will see if that does the trick with your changes 🙂

Btw I really like the approach you took on Incendiary-iguana to abstract out the common controller code.

Glad to hear it 😄 Hope we can bring some of the same goodies over here

Copy link
Contributor

@frodopwns frodopwns left a comment

Choose a reason for hiding this comment

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

Need to merge the latest code then when the tests pass we are good to go on this one

@alexeldeib
Copy link
Contributor Author

thanks for rebasing @frodopwns! I'll wait for a final okay but this looks good to merge?

@alexeldeib
Copy link
Contributor Author

whoops, I confused this with the other PR that had more functional changes -- that still needs a rebase, but this one is cosmetic so I will go ahead and merge 👍

@alexeldeib alexeldeib merged commit 6abdd8b into Azure:master Oct 15, 2019
Porges added a commit that referenced this pull request May 11, 2021
This installs the tools to `$clone/hack/tools` instead. There is a `dev.sh` script to run to get into a shell with a `PATH` pointing to the right place.
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.

3 participants