-
Notifications
You must be signed in to change notification settings - Fork 89
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
Feature/add pipeline variables #11
Feature/add pipeline variables #11
Conversation
@nishubansal @N-Usha this is a much needed feature for this Action. Can we get this reviewed and merged in once the |
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.
LGTM ... @nishubansal @N-Usha for final review and merging
502e88f
to
b4d74ec
Compare
@ionutleca is there any progress on the PR ? This feature would be very usefull |
So it looks like this PR is stuck in limbo. Is there some other way of getting this functionality into here? Being able to set the values of pipeline variables is really important. Can the core team just implement the functionality without using this PR? Or can I submit a PR with a similar code to get this moving? |
I will try to get my current employer approval for the CLA required for this PR. If I find out that it takes more than 24h, I'll let you know if you should just create the PR with these changes yourself, @mfcollins3. |
Thanks, @ionutleca. I greatly appreciate you getting the CLA approved. I can't wait to start using this! |
@ionutleca I'm assuming since 24h turned into 2+ months, one of us can go ahead and create the PR with these changes ourselves? |
The CLA was approved a long time ago. |
Apologies, the timeline was a bit confusing. It’s a shame because this is super useful, but they seemed to let it fall by the wayside. |
No worries. It seems useful to me as well, but we might be the only ones :)) |
@nishubansal @N-Usha @anweiss Can anyone finally get this merged in? |
Any chance either this or the PR for adding build parameters might get merged in? |
+1 to getting this merged in |
b4d74ec
to
e6014dc
Compare
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.
Tested this and using this solution in a fork of this repo for triggering an ADO release pipeline with variables that needed set. Approving but one more small change needed where the js file needs rebuilt to include the latest change 👍
e6014dc
to
a4321d6
Compare
* Add pipeline variables * Better variables handling
No description provided.