-
Notifications
You must be signed in to change notification settings - Fork 6
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 suffix input and support empty strings in prefix/suffix #333
Conversation
c471618
to
e314ed0
Compare
94c160f
to
3b35bd5
Compare
7643895
to
1cdfafe
Compare
@manics sorry for pinging for review and pushing changes after, I won't touch this further now though. |
The idea seems fine. Have you thought about using |
Yes! I found an issue is that we had a whitespace and/or comma deliminated list for prefix, so doing that would be weird there. I looked to use the same syntax for suffix/prefix, and to not make a breaking change. I also concluded that if i made use of new lines as separators for suffix, it would end up making users write hard-to-write-correctly and hard-to-understand yaml config. |
I meant (basically) suffix: |
""
-tigervnc suffix: "", -tigervnc |
Ah, okay - we can do that also! I'll update this PR to reflect that when i'm at a computer next time! |
@manics done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good!
The new parameter is missing from the README:
https://github.com/consideRatio/action-major-minor-tag-calculator/tree/pr/add-suffix-input?tab=readme-ov-file#optional-input-parameters
Thank you @manics, great catches! |
95cf9b0
to
d0b4418
Compare
Force pushed away repeated |
Adds a
suffix
input and supports specifying an empty string element in a list of prefixes/suffixes via eitherprefix
orsuffix
input by specifying""
instead.Having
suffix
next toprefix
is probably generally useful, but I have a use case for jupyterhub/jupyter-remote-desktop-proxy to publish the same image with the suffixes""
and"-tiger"
as planned via jupyterhub/jupyter-remote-desktop-proxy#88.This is an example of how I'd use it there: