Skip to content

Conversation

@jimson-msft
Copy link
Contributor

  • Tested this on a dry-run and release now contains 4 tar files respective to each platform
  • It's a little messy the way this pipeline does the .tar file creation so that can be cleaned up in the future

@jimson-msft jimson-msft requested a review from a team as a code owner October 4, 2021 16:52
@jimson-msft
Copy link
Contributor Author

@shishirb-MSFT If this change looks good we can also CP it into main and re-release 0.8.0 with it. I think the current release is a little ambiguous on which .deb file to use per platform

jobs:
- job: ${{parameters.targetOsArch}}
steps:
- checkout: self
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Oct 4, 2021

Choose a reason for hiding this comment

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

checkout

Do we need the checkout step at all? Isn't 'self' the default? #Closed

Copy link
Contributor Author

@jimson-msft jimson-msft Oct 6, 2021

Choose a reason for hiding this comment

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

That's what I thought too - but after templatization having nested templates was causing issues - the 2nd stage in the github-release.yml wasn't performing checkout unless explicitly specified. It's unclear if it's a bug or a feature, though I did see some people posting about similar issues online, but they look a bit dated:

https://developercommunity.visualstudio.com/t/azure-devops-pipeline-with-template-does-not-check/579283

steps:
- checkout: self
path: 's' # Copy to sources directory (templates do not do this automatically)
- template: ../../build/linux/du/templates/doclient-lite-docker-steps.yml
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Oct 4, 2021

Choose a reason for hiding this comment

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

template

Please add a blank line between template/task/job lines. #Resolved

Copy link
Collaborator

Choose a reason for hiding this comment

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

In all yamls here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I kept them without the line to make it more obvious that this task is related to the previous job

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Prefer the blank line. What about lines 16, 60, 66, 73?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh gotcha, misunderstood the ask - thought you meant only adding spaces between jobs and tasks. Added the additional spaces

*.deb
TargetFolder: '/tmp/${{parameters.targetOsArch}}'
CleanTargetFolder: true
displayName: 'Copy all agent .deb file'
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Oct 4, 2021

Choose a reason for hiding this comment

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

all

nit: remove 'all' #Closed

- task: PublishBuildArtifacts@1
inputs:
PathtoPublish: '$(Build.ArtifactStagingDirectory)'
ArtifactName: '${{parameters.targetOsArch}}'
Copy link
Collaborator

@shishirb-MSFT shishirb-MSFT Oct 4, 2021

Choose a reason for hiding this comment

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

${{parameters.targetOsArch}}

Can we omit this name so that the .tar file shows up in the root folder of artifacts? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good to me

Copy link
Contributor Author

@jimson-msft jimson-msft Oct 6, 2021

Choose a reason for hiding this comment

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

Well, it is a required arg, so I'm going to modify publishing all .tar files to a folder called release-artifacts since we are no longer publishing the .deb files directly

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok we'll see if it complains due to multiple jobs writing to the same artifact name.

@jimson-msft jimson-msft merged commit 8ad2ce1 into develop Oct 7, 2021
@jimson-msft jimson-msft deleted the user/jimson/tar_release_pipeline branch October 7, 2021 00:12
shishirb-MSFT added a commit that referenced this pull request Oct 7, 2021
Co-authored-by: Jimmy Song <55897143+jimson-msft@users.noreply.github.com>
Cherry-picked from #89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants