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

Add OpenVSX publishing #1064

Merged
merged 3 commits into from
Nov 24, 2022
Merged

Conversation

filiptronicek
Copy link
Contributor

@filiptronicek filiptronicek commented Apr 20, 2022

This is a succession to #388, which adapts publishing to OpenVSX embedded in the publishing workflow in GitHub Actions.

Fixes #379

Before using this it is necessary to set up an OVSX_PAT GitHub Action secret with the value from open-vsx.org/user-settings/tokens. A claim of the hashicorp namespace will be also required (this can be done by submitting an issue in the OpenVSX repo).

@filiptronicek filiptronicek requested a review from a team as a code owner April 20, 2022 13:31
@hashicorp-cla
Copy link

hashicorp-cla commented Apr 20, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Hi @filiptronicek
Thanks for the PR! It's great to see OpenVSX now supports multi-platform extensions.

We'll get the process started for (re)claiming the hashicorp namespace and generating the PAT.

In the meantime, I have just two questions:

  1. Would you mind signing the CLA?
  2. Would you mind decoupling the OpenVSX into a separate job? i.e. we now have publish job, so we could rename it to publish-ms-marketplace & name: Publish to VS Code Marketplace and then add another job, practically identical, just with different secret and npx vsce replaced for ovsx publish.

The reason for coupling is:

  • if the OpenVSX job fails, we don't want to be in a situation where we have to re-publish to the Marketplace too
  • the two publishing jobs can run in parallel

@filiptronicek
Copy link
Contributor Author

Hey @radeksimko, thanks for the quick review!

Would you mind signing the CLA?

Working on that :)

Would you mind decoupling the OpenVSX into a separate job?

I would not at all, but maybe we could implement something a bit different which would wouldn't require its own step. We could essentially have the setup I made, but we would use if: always() on the OpenVSX step so that these two registries publish independently without having a successful publish to the other one.

the two publishing jobs can run in parallel

This is a fair point that would not work with the approach I'm proposing, but I think the ~30 seconds it takes the job to publish all of the extension versions could be tolerated because the GitHub Actions runners often have different boot times (hence this approach could be faster sometimes, and sometimes slower).

@radeksimko
Copy link
Member

So the main benefit of splitting it into its own job is that GitHub allows us to rerun a failed job, but we cannot rerun individual steps unfortunately.

Screenshot 2022-04-20 at 16 25 30

So let's say that OpenVSX publishing fails and Marketplace succeeds - if we had both as steps - we'd have to retry both, or have some extra logic to ensure that repeated publishing to the Marketplace is harmless. More likely we just wouldn't retry.

but we would use if: always() on the OpenVSX step so that these two registries publish independently without having a successful publish to the other one.

Right, I wasn't entirely clear but this is the main benefit I had in mind when talking about parallel jobs, more than the time saved. I think the two jobs can still be independent of each other and both use something like this?

    needs: build
    if: success() && startsWith( github.ref, 'refs/tags/v')

@filiptronicek
Copy link
Contributor Author

filiptronicek commented Apr 20, 2022

I think the two jobs can still be independent of each other and both use something like this?

I think so as well.

All other comments sound fair, I will add it as a separate job just like the MS publish. My main concern here was just duplicate workflow code, but that could perhaps be addressed in a future PR with moving some steps into their own .yml files.

Update: just realized there will be close to no duplicate code, I thought we would have to create a whole new workflow 🤦 😄.

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

This LGTM, thank you for the edits and for signing the CLA.

I'm just in the process of clarifying one small part of the Terms with the Eclipse Foundation, as asked by our Legal team and once done I will go ahead with the namespace transfer and PAT creation, and then we can merge this.

Thanks for your patience.

@radeksimko radeksimko self-assigned this Apr 20, 2022
@radeksimko radeksimko added the ci Continuous integration/delivery related label Apr 20, 2022
filiptronicek added a commit to manuth/publish-extensions that referenced this pull request Apr 21, 2022
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

@filiptronicek We have claimed the namespace and have a PAT ready, but there's still an issue with publishing as the latest version of ovsx with the target support isn't available on npm - see eclipse/openvsx#450

Not realizing I used an old version (0.3.0) I managed to accidentally already publish two versions as universal (2.20.0 & 2.20.1) - is there any way of either retracting them or republishing as platform specific?

Co-authored-by: Radek Simko <radek.simko@gmail.com>
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

All issues regarding platform-specific extensions on the OpenVSX side seem to have been resolved. I was able to upload all the missing versions manually. 🎉

After one more minor change, we can finally merge this PR. Thank you for being so patient.

@dbanck dbanck requested a review from radeksimko November 24, 2022 08:42
@dbanck dbanck merged commit c12c96f into hashicorp:main Nov 24, 2022
@filiptronicek filiptronicek deleted the ft/publish-to-openvsx branch November 24, 2022 18:04
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci Continuous integration/delivery related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish extension on open-vsx.org
4 participants