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

Adding extra annotations to service #150

Merged
merged 8 commits into from
Jan 12, 2022
Merged

Adding extra annotations to service #150

merged 8 commits into from
Jan 12, 2022

Conversation

matthewkrupnik
Copy link
Contributor

Proposed changes

Currently there is a way to add extra label to the nginx-ingress-controller service, but not annotation.
Why annotations? One example is when using MetalLB if you want services to share same IP you have to add an annotation for that.

This would likely be useful for a very small amount of people, but if operator allows extra labels, why not also support annotations?

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@ciarams87 ciarams87 added the enhancement Pull requests for new features/feature enhancements label Aug 17, 2021
@matthewkrupnik
Copy link
Contributor Author

What's the reluctance with merging this PR? It's been sitting around for 3 months with no feedback at all. All tests are passing, change in minor and there are no comments good or bad. What do I need to do to get some eyes on this?

Copy link
Contributor

@lucacome lucacome left a comment

Choose a reason for hiding this comment

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

Hi @matthewkrupnik, sorry for the long wait. LGTM!

@lucacome lucacome merged commit e29cca1 into nginxinc:master Jan 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants