Skip to content

Conversation

@jjw24
Copy link
Member

@jjw24 jjw24 commented Jul 26, 2021

  • Fix portable download link in readme

  • Fix WinGet deployment so it occurs only on non-pr master branch builds.

  • Change Nuget publish to tag only

@jjw24 jjw24 added the bug Something isn't working label Jul 26, 2021
@jjw24 jjw24 added this to the 1.8.1 milestone Jul 26, 2021
@jjw24 jjw24 force-pushed the fix_readme_ci branch 2 times, most recently from 6e6c365 to 755a687 Compare July 26, 2021 11:09
@jjw24 jjw24 requested review from JohnTheGr8 and taooceros July 26, 2021 11:16
@jjw24 jjw24 self-assigned this Jul 26, 2021
@jjw24 jjw24 enabled auto-merge July 26, 2021 11:16
@taooceros
Copy link
Member

Does the winget update work? The check fail on last CI run

Copy link
Member

@JohnTheGr8 JohnTheGr8 left a comment

Choose a reason for hiding this comment

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

Minor issue: I think it would be better to check APPVEYOR_REPO_TAG instead, so that the deployment to WinGet happens from the same CI build as the release deployment to Github

@jjw24
Copy link
Member Author

jjw24 commented Jul 26, 2021

Does the winget update work? The check fail on last CI run

you mean the ci on the release pr? then yeah update will work because it's just failed to push the update.

@jjw24 jjw24 requested a review from JohnTheGr8 July 26, 2021 23:14
@jjw24
Copy link
Member Author

jjw24 commented Jul 26, 2021

Minor issue: I think it would be better to check APPVEYOR_REPO_TAG instead, so that the deployment to WinGet happens from the same CI build as the release deployment to Github

Done

On a side note @JohnTheGr8 why are we not publishing to Nuget on tag as well? I see the build history of the 1.8.0 release tried to publish to nuget on tag build too

@JohnTheGr8
Copy link
Member

JohnTheGr8 commented Jul 27, 2021

Done

Hmm, I think this:

if ($env:APPVEYOR_REPO_BRANCH -eq "master" -and $env:APPVEYOR_REPO_TAG)

is equivalent to this:

if ($env:APPVEYOR_REPO_BRANCH -eq "master")

because it only checks if $env:APPVEYOR_REPO_TAG is set and not its value (appveyor always sets APPVEYOR_REPO_TAG)... I think we need to do it like this:

if ($env:APPVEYOR_REPO_BRANCH -eq "master" -and $env:APPVEYOR_REPO_TAG -eq "true")

On a side note @JohnTheGr8 why are we not publishing to Nuget on tag as well? I see the build history of the 1.8.0 release tried to publish to nuget on tag build too

I don't think there's a reason, we initially did nuget deployment on master builds and didn't switch to tag builds when we added github deployments later on...

@jjw24 jjw24 changed the title Fix portable readme download link & WinGet pipeline Fix portable readme download link & WinGet pipeline & change Nuget publish on tag only Jul 27, 2021
@jjw24
Copy link
Member Author

jjw24 commented Jul 27, 2021

@JohnTheGr8 Fixed

@jjw24 jjw24 merged commit 995d78b into dev Jul 28, 2021
@jjw24 jjw24 deleted the fix_readme_ci branch July 28, 2021 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants