Skip to content

Conversation

@marufrasully
Copy link
Contributor

@marufrasully marufrasully commented Jan 24, 2023

Create a simple bas extension. Required data can be seen here. These data will be copied over when yarn ci command is executed

Note: This will be the only public module on https://www.npmjs.com/

console.log("Copying finished successfully");

const vsixFiles = [];
readdirSync(getBaseSrc()).forEach((item) => {
Copy link
Member

Choose a reason for hiding this comment

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

If you are going to copy only one file
Why do you scan and collect all *.vsix file names?

And in this case should not it be an error if multiple *.vsix files are found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because file name e.g vscode-ui5-language-assistant-3.3.1.vsix is not clear und therefore checks base folder to get one.

Regarding multiple vsix files, it is usually never the case on CI, but anyway I adapted logic which throws an error in case of multiple or when found not vsix file.

Copy link
Member

@bd82 bd82 left a comment

Choose a reason for hiding this comment

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

metadata (package.json) of this new package needs more cleaning imho.
also please define a .gitignore in this new package so copied artifacts (*.vsix)
will not make changes in git working tree.

);
}
const vsixFile = vsixFiles.pop();
if (!vsixFile) {
Copy link
Member

Choose a reason for hiding this comment

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

you can probably combine these two checks with vsixFiles.length === 1

But this is not mandatory...

@marufrasully marufrasully merged commit de4eb8a into master Jan 25, 2023
@marufrasully marufrasully deleted the feat/create-simple-bas-ext branch January 25, 2023 14:29
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.

4 participants