-
Notifications
You must be signed in to change notification settings - Fork 12
Support packageTask, packagePath, packageUploadOverrides for complibs #282
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
Support packageTask, packagePath, packageUploadOverrides for complibs #282
Conversation
…omponentLibraryConfiguration
TwitchBronBron
left a comment
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.
overall, this is a really nice and clean PR. I've sent you a few examples of how to write some unit tests for this in slack, would appreciate seeing some for each of the changed code here. but once those are in, this looks good to go!
Don't forget, we'll also need to open a PR in https://github.com/rokucommunity/vscode-brightscript-language to add support for these new properties in the componentLibraries launch.json json schema. (like we did in this PR)
| to: (match: string) => { | ||
| // only alter file ending if it is a) pkg:/ url or b) relative url | ||
| let isPkgUrl = !!/^uri\s*=\s*"pkg:\//i.exec(match); | ||
| let isPkgUrl = !!/^uri\s*=\s*"(pkg:|libpkg:)\//i.exec(match); |
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.
Can you make sure to add a few unit tests around this to make sure we are capturing pkg: as well as libpkg:?
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.
The PR to vscode-brightscript-language has been opened rokucommunity/vscode-brightscript-language#675
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've extended the existing src/managers/ProjectManager.spec.ts tests to also check libpkg: and added unit tests for packageTask, packagePath and packageUploadOverrides
TwitchBronBron
left a comment
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.
The tests look great. Thanks!
Add support for
packageTask,packagePath,packageUploadOverridesproperties forcomponentLibrariesentries in launch options. This provides additional flexibility for configuring the channel app project if it relies on a componentLibrary built with a custom(e.g.make) script in thepackageTask