-
Notifications
You must be signed in to change notification settings - Fork 37
Improve contributing guide and workflow #114
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
Conversation
Removes the indirection of the JS files, and runs commands straight from package.json instead
tsc has been deprecated in favor of typescript
The vsix extension should only be built after the changelog commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
@@ -6,7 +6,7 @@ NEW_VERSION=${TRAVIS_TAG#"v"} | |||
|
|||
# Build the extension | |||
yarn install | |||
yarn build | |||
yarn build:syntax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is yarn build:extension
executed? Is that part of yarn vscode:publish
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. From the vsce docs it seems that yarn vscode:publish
(aka vsce publish
) does the packaging before publishing, as it is the only command needed to publish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
"ts-node": "^8.10.2", | ||
"tsc": "^1.20150623.0", | ||
"typescript": "^3.9.5", | ||
"vsce": "^1.77.0", | ||
"vscode-tmgrammar-test": "0.0.10" | ||
}, | ||
"scripts": { | ||
"vscode:prepublish": "test -f ./syntaxes/Scala.tmLanguage.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this prepublish
script is actually doing anything useful? I tried messing with the json file but this doesn't result in failures of vscode:prepublish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaximeKjaer , it just checks if the file ./syntaxes/Scala.tmLanguage.json
exists.
This PR:
package.json
,This should simplify the workflow little, as there is a Yarn task for every possible action. The basic workflow now only consists of
yarn install
,yarn build
andyarn test
. But you can still run very more precise tasks throughyarn test:unit
oryarn build:extension
, for instance.