This is an archive of the discontinued LLVM Phabricator instance.

[clangd][vscode] Add npm helper commands to package/release the extension.
ClosedPublic

Authored by hokein on Sep 26 2019, 4:44 AM.

Event Timeline

hokein created this revision.Sep 26 2019, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2019, 4:44 AM
ilya-biryukov added inline comments.Sep 26 2019, 5:21 AM
clang-tools-extra/clangd/clients/clangd-vscode/package.json
38

Just to make sure I understand what's going on...

Could you explain what --baseImagesUrl does?
The suggested url returns an error code when I try to load it in a browser.

hokein marked an inline comment as done.Sep 26 2019, 6:41 AM
hokein added inline comments.
clang-tools-extra/clangd/clients/clangd-vscode/package.json
38

This is expected, this is a base url. it is used to assemble a full url with the screenshot links in the readme, an example is https://raw.githubusercontent.com/llvm/llvm-project/master/clang-tools-extra/clangd/clients/clangd-vscode/ + doc-assets/complete.png, so that the screenshot can be rendered correctly in the VSCode market.

ilya-biryukov accepted this revision.Sep 26 2019, 6:50 AM

LGTM

clang-tools-extra/clangd/clients/clangd-vscode/package.json
38

Thanks for the clarification! Surprised we need this for both package and publish, I'd expect this to be handled in either first or the second step, but not both. I guess I don't fully understand what they do with this URL.

NIT: could add a comment mentioning that this URL is used to "host images (e.g. screenshots in documentation)"?

This revision is now accepted and ready to land.Sep 26 2019, 6:50 AM
hokein marked an inline comment as done.Sep 26 2019, 7:00 AM
hokein added inline comments.
clang-tools-extra/clangd/clients/clangd-vscode/package.json
38

Thanks for the clarification! Surprised we need this for both package and publish, I'd expect this to be handled in either first or the second step, but not both. I guess I don't fully understand what they do with this URL.

Actually, when we make a new release, we use publish which includes the package. The package is usually used to build a local package (mainly for debugging purpose).

NIT: could add a comment mentioning that this URL is used to "host images (e.g. screenshots in documentation)"?

Unfortunately, comment is not permitted in the JSON :(

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2019, 7:02 AM