-
Notifications
You must be signed in to change notification settings - Fork 105
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
Add descriptor visibility filter #112
Conversation
My concern with these is that the visibility filter is saying "These must not be shown to anyone" whereas the truth is that they should be an option for vSphere cloud-provisioned slaves (but only vSphere cloud-provisioned slave nodes). ...that said, I think this would only ever become a problem if If you agree with that, and you're happy for this to be merged, please add a PR description and I'll merge it. |
I suspect an end-of-line issue - the diff is now showing you've replaced the entire contents of both files... |
😱 I just used GitHub to merge a simple conflict 😭 |
Of course, fixing the EOLs in #114 has meant this now has merge issues ... again 😀 |
i'll double check on the visibility but I recall them as being visible and selectable :) |
Thank you for your perseverance 👍 |
PS. It's the end of my working day now, but if you find any other issues, comment/raise-a-PR etc and I'll be back online in around 15 hours. |
For a cloud-provisioned slave, none of that should be editable anyway... They should be edited by configuring the template that defines how they're created, but that's (probably) a bug unrelated to your changes - one of many minor bits of tech-debt in this plugin code. The important things are that:
|
No description provided.