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.

Diff Detail

Repository
rL LLVM

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 ↗(On Diff #221921)

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 ↗(On Diff #221921)

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 ↗(On Diff #221921)

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 ↗(On Diff #221921)

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