Skip to content
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.

Sign up for GitHub

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

[FIX] JSDoc: add project name to JSDoc configuration and api.json #609

Merged
merged 1 commit into from
Jun 3, 2021
Merged

[FIX] JSDoc: add project name to JSDoc configuration and api.json #609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/processors/jsdoc/jsdocGenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ async function generateJsdocConfig({targetPath, tmpPath, namespace, projectName,
"ui5": {
"variants": ${JSON.stringify(variants)},
"version": "${version}",
"uilib": "${projectName}",
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not use the projectName (= ui5.yaml metadata name) but the namespace.
Usually they are equal but the namespace is guaranteed to be correct as it is also used for the folder structure. The project name is mainly used for logging purposes.

See my abandoned change here: https://github.com/SAP/ui5-builder/pull/598/files#diff-03d08225d447cd18da349dd9928b9093a83e66d9a6aa66cee4e0a0038779a9c8

Copy link
Member

Choose a reason for hiding this comment

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

Hm, what does "correct" mean here anyways? If the "uilib" property shall contain the libraries name, the projectName is always correct imho.

Copy link
Member

Choose a reason for hiding this comment

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

Especially for that "library" check IMO it's important that it fits to the namespace of the library. If it would be something else (e.g. mylib instead of com.example.mylib), the check would fail and report false findings.

Copy link
Member

Choose a reason for hiding this comment

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

I see. In that case we should change it to the namespace

"jsapiFile": "${jsapiFilePath}",
"apiJsonFolder": "${apiJsonFolderPath}",
"apiJsonFile": "${apiJsonFilePath}"
Expand Down
2 changes: 1 addition & 1 deletion test/expected/build/library.j/dest/test-resources/library/j/designtime/api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"$schema-ref":"http://schemas.sap.com/sapui5/designtime/api.json/1.0","version":"1.0.0","symbols":[{"kind":"namespace","name":"library.j","basename":"j","resource":"library/j/some.js","module":"library/j/some","static":true,"visibility":"public"}]}
{"$schema-ref":"http://schemas.sap.com/sapui5/designtime/api.json/1.0","version":"1.0.0","library":"library.j","symbols":[{"kind":"namespace","name":"library.j","basename":"j","resource":"library/j/some.js","module":"library/j/some","static":true,"visibility":"public"}]}
1 change: 1 addition & 0 deletions test/lib/processors/jsdoc/jsdocGenerator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ test("generateJsdocConfig", async (t) => {
"ui5": {
"variants": ["apijson"],
"version": "1.0.0",
"uilib": "some.namespace",
"jsapiFile": "${jsapiFilePath}",
"apiJsonFolder": "${apiJsonFolderPath}",
"apiJsonFile": "${apiJsonFilePath}"
Expand Down