Skip to content
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

fix: publish win32 and darwin artifacts to release assets #474

Closed
wants to merge 2 commits into from

Conversation

isha689
Copy link
Contributor

@isha689 isha689 commented Oct 18, 2022

Fixes DII-694

The darwin and win artifacts which were previously hosted on aws-s3, will now be published to github release assets. Homebrew and scoop repositories will have relevant changes to pick up the release assets.

Refer the 5.1.10 and 5.1.10-draft.2 releases to verify the changes. Changes made in scoop and homebrew repos to refer the release assets instead of aws-s3 bucket.
scoop-5.1.10 and scoop-5.1.10-draft.2
homebrew-5.1.10 and homebrew-5.1.10-draft.2

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

core.setFailed(`Exiting, required param: ASSET_NAME is missing`);
return;
}
if(!('FILE' in process.env) && !('DIRNAME' in process.env)){
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an OR condition / AND ? The error message says OR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to modify the error message.

})
release_id: oldReleaseId
});
const duplicate_asset = assets.data.find(a => a.name === assetName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any sample runs to validate duplicate_asset logic block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, refer this: Run 1 -> uploaded assets successfully.
Run 2 -> uploaded the same assets again, because overwrite flag has been set in oclif-release workflow.

@@ -25,8 +25,8 @@ jobs:
matrix:
include:
- os: macos-latest
asset_name: twilio-v${{ github.event.inputs.tag-name }}
command_name: npx oclif-dev pack --xz -t darwin-arm64,darwin-x64,linux-arm,linux-x64,win32-x64,win32-x86
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to remove the build targets too? IIRCtwilio update refers the vanilla builds from s3. How does it work after this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I missed that. Thanks!!

id: sha256
if: ${{matrix.publish == 'homebrew'}}
run: |
make clean
make install
npx oclif-dev publish
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't we still need to publish the darwin tarballs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Darwin tarballs will only be needed by twilio update cmd, which can fetch them successfully from the release assets. I don't think if there's a need to publish them to s3 bucket.

@@ -148,7 +148,21 @@
"update": {
"s3": {
"xz": true,
"bucket": "twilio-cli-prod"
Copy link
Contributor

Choose a reason for hiding this comment

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

How are we publishing apt artefacts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That needs to be handled differently as part of buildkite job.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

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