-
Notifications
You must be signed in to change notification settings - Fork 22
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
[FEATURE] Add new theme-library type #285
Conversation
|
Theme libraries do not require as many features as normal libraries. For example they do not need a namespace. Also many build tasks are not required. Also replace copyright strings in .less and .theme files.
0534019
to
ea87e85
Compare
Moved the previous changes in LibraryFormatter to #392 |
As discussed, release this together with a new specification version "1.1" to give developers a good error message when consuming updated OpenUI5 theme libraries. For this, the theme-library formatter needs to be enhanced to check for spec version 1.1 or higher. Similar to what has been done in proxy PoC: https://github.com/SAP/ui5-project/pull/209/files#diff-bf8943f684859f68f7e7a4d3cfed68ddR275 |
TODO: Update relevant documentation |
return Promise.resolve().then(() => { | ||
if (!project) { | ||
throw new Error("Project is undefined"); | ||
} else if (project.specVersion === "0.1" || project.specVersion === "1.0") { |
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.
@RandomByte do you think this simple check is okay? With this we rely on the general specVersion check within ui5-project to be executed, otherwise also other specVersions would be allowed.
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.
LGTM for the time being
The new 'theme-library' type requires spec version 1.1: SAP/ui5-builder#285
The new 'theme-library' type requires spec version 1.1: SAP/ui5-builder#285 Also change wording of the error message for unsupported spec versions and fix the referenced URL. This should make it clear that the used version of the UI5 CLI might be outdated.
The new type 'theme-library' requires this spec version 1.1. See SAP/ui5-project#252 and SAP/ui5-builder#285
The new 'theme-library' type requires spec version 1.1: SAP/ui5-builder#285 Also change wording of the error message for unsupported spec versions and fix the referenced URL. This should make it clear that the used version of the UI5 CLI might be outdated. Configuration.md: Add deep link to Specification Version documentation
}, | ||
|
||
// Export type classes for extensibility | ||
Builder: ThemeLibraryBuilder, |
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.
does "for extensibility" only mean that consumers can extend those classes or does it also mean that they can inject own implementations here via these two names? If so, the new expressions above should use the properties of the export, not the import names.
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, subclassing
The new type 'theme-library' requires this spec version 1.1. See SAP/ui5-project#252 and SAP/ui5-builder#285
Theme libraries do not require as many features as normal libraries.
For example they do not need a namespace. Also many build tasks are
not required.