huserben / TfsExtensions

Extensions for TFS 2015+ such as custom Widgets (require TFS 2017) and Build Tasks
MIT License
44 stars 22 forks source link

Expose `TriggeredBuildIds` as multi-job/stage variable #208

Closed ChristoWolf closed 1 year ago

ChristoWolf commented 2 years ago

Hi @huserben!

Your Trigger Build Task guide on the VS marketplace specifies for the TriggeredBuildIds variable, that

The variable is as well available as an input in the configuration for any subsequent Task.

However, unless I missed something, this only holds true for tasks in the same job.

I'll try to propagate the variable as a multi-job variable on my own for now, but I think it would be useful if it was exposed to the whole build in general by default or through an option.

huserben commented 2 years ago

Hi @ChristoWolf

I think you are right, it's currently just available for tasks in the same job.

When a new update is coming I can try to check where I would need to make a change so it could also be available for tasks in following jobs.

In the meantime one way you could use would be to use a powershell script (or similar) and make it available like that (as I can't give you any ETA on when I'll get to do some update).

The docs should give some hint, but probably something like this would work (this is code from the top of my head - not tested so there might be errors...):

- pwsh: |
    Write-Host "##vso[task.setvariable variable=TriggeredBuildIdsGlobal;isoutput=true]$env:TriggeredBuildIds"
  name: SetVars

If everything works correctly the variable TriggeredBuildIdsGlobal should then be available for subsequent tasks in any following job via $(SetVars.TriggeredBuildIdsGlobal)

ChristoWolf commented 2 years ago

Hi @huserben!

Thanks for the quick reply!

One then still also has to inject the variable into the job then, but yes, that is exactly what I have done. Works perfectly fine, I just thought to let you know that the guide might be misunderstood.

Anyways, thanks for these great extensions, very useful stuff!

huserben commented 2 years ago

One then still also has to inject the variable into the job then, but yes, that is exactly what I have done.

What exactly do you mean by "inject into the job"? As I think what could be done is to make the powershell code I posted part of the task, but you would still need to reference it via the same way in the next job, I think no way to work around this.

ChristoWolf commented 2 years ago

Sorry for being unclear. The output/multi-job variable cannot simply be referenced in follow-up jobs without injection via expressions. It needs to be set/injected accordingly to be usable.

huserben commented 2 years ago

Sorry for the long wait, I missed that you answered so fast :-)

Yes, I think there is no other way (technically from Azure Pipelines), or is there? :-)

I'm planning on creating a new version in the near future, so what I could do is the following:

Am I missing something or would you expect something else?

Thanks for your support

ChristoWolf commented 2 years ago

Hey @huserben!

No worries, you responded quickly anyways :)

IMO, the first change regarding the automatic propagation as an output variable is not really needed. That would make built-in Azure Pipelines behavior hidden as magic in your task, and I think explicitness would be better here, i.e. that the consumer needs to manually promote the variable, as they also need to then inject it into their job of choice.

However, I do absolutely agree that the docs should reflect this, i.e. that the consumer needs to make explicit use of Azure Pipelines' usual multi-job/multi-stage variables mechanism to use TriggeredBuildIds outside of the current job/stage.

But these are just my opinions, of course feel free to disagree/agree wherever :)

huserben commented 2 years ago

Hi @ChristoWolf

I just published a new version of the task with an updated documentation. I took the very easy road and more or less added your statement from above together with links to the docs from microsoft and this issue: Mulit-Job Variable Usage Doc

Do you think this will be sufficient? Or should something else be added?

Thanks for your support.

ChristoWolf commented 2 years ago

Hi @huserben!

Thanks for all the quick effort, very appreciated!

The doc addition looks great, the only I thing I think would still make sense to change/update would be the usages of available to all following tasks (paraphrasing), as that might still be a bit misleading when not reading the full doc.

But again, please feel free to disagree!

Also, thanks for listing me as a contributor in the release, really wasn't necessary, but I appreciate it :)

huserben commented 2 years ago

Hi @ChristoWolf

thanks for spending time on a sunday to give me feedback. You are right, I did not think about adjusting this, but it certainly makes sense. I'll update this soon. Sadly the docs (on the marketplace) are only updated with a new release, so technically I have to release a new version with just the doc change - I'm not sure whether I should do this or just have it "bundled" with the next set of changes - I'll leave the issue open till I've updated the docs in github at least and made a decision on whether I release a new version or not.

I do highly appreciate people taking the time to write here in order to improve the task, this will help others using the task as well. So most certainly you (and everyone else raising issues) is contributing and therefore deserves a mention :-)

ChristoWolf commented 2 years ago

Hi @huserben!

Well, sorry for disrupting your weekend! Sounds great, thanks again!

huserben commented 1 year ago

Hi @ChristoWolf

finally found the time to do some pending updates of the documentation.

I've now added it like this: image

I hope that makes it a bit more clear for the readers in future. Do you think it's fine now and we can close this issue or do you see other things that are still pending?

ChristoWolf commented 1 year ago

Sounds perfect, thanks @huserben!