Skip to content

Conversation

@filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Mar 1, 2022

This PR introduces a custom property available to extensions to let them have an entirely customized process, because not all extensions are built the same, meaning even some of the biggest ones we have are failing.

This PR adds a very powerful option, in docs we should make it clear that this should be used as a last measure (we need to favor extensions with no setup, if they require some then we should point to the prepublish setting).

How to test

Try to execute the following to test that everything is working correctly:

EXTENSIONS=formulahendry.auto-rename-tag FORCE=true GITHUB_TOKEN=YOUR_PAT node publish-extensions

@akosyakov
Copy link
Collaborator

What is the motivation?

@filiptronicek
Copy link
Member Author

filiptronicek commented Mar 1, 2022

@akosyakov mostly testing, but it should fix publishing some harder Microsoft extensions, specifically https://github.com/microsoft/vscode-js-profile-visualizer, because it requires the deps install to never occur, because if you do npm install in the subfolder of the cloned repo it fails. I tried going around it yesterday but it seems pretty tricky, hence this. I'm sure it will also be helpful for other extensions as well, but if we can fix these exts without such an aggressive option, I'm pretty sure it would be better.

Edit: this now also fixes https://github.com/formulahendry/vscode-auto-rename-tag, which has never successfully published to OpenVSX (8M downloads)

@akosyakov
Copy link
Collaborator

I just wonder we already have prepublish and extensionFile and we can change location should not it be enough?

@filiptronicek
Copy link
Member Author

Actually, using the extensionFile hasn't occured to me, that's a pretty nifty idea!

Sadly, two issues arise with this:

Firstly, the extension also requires a yarn flag to be passed to vsce to build properly as well, without having a yarn.lock or any yarn config.

Secondly, the manifest path is derived from the location, which in this case is not desirable, since the location would be the root of the repo and that is just lerna config and dependencies.

@filiptronicek filiptronicek linked an issue Mar 3, 2022 that may be closed by this pull request
Copy link
Contributor

@neurolag neurolag left a comment

Choose a reason for hiding this comment

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

Looks awesome to me - I'm looking forward to have the profiler (and stuff like jupyter) running in Codium at some point! 😄

There's one small change I'd like to see in this PR

Copy link
Contributor

@neurolag neurolag left a comment

Choose a reason for hiding this comment

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

Hope you don't mind me interrupting once more. I think it might be a good idea to let the devs choose whether to provide a custom array or a string.

Looks great 😄

Co-authored-by: Manuel Thalmann <m@nuth.ch>
@filiptronicek filiptronicek marked this pull request as ready for review March 10, 2022 20:00
@filiptronicek filiptronicek merged commit 2ac66d9 into master Mar 24, 2022
@filiptronicek filiptronicek deleted the ft/add-custom-option branch March 24, 2022 16:47
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.

Publish vscode-js-profile-table

4 participants