-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Conversation
package.json
Outdated
@@ -11,7 +11,8 @@ | |||
"deploy": "scripts/deploy.sh", | |||
"test": "(cd dist; python test_runner.py ${CLOUD_SDK-~/google-cloud-sdk} ../tests/)", | |||
"generate-api-docs": "cd scripts && node generate_api_docs_2.js", | |||
"generate-api-docs-1": "cd scripts && node generate_api_docs_1.js" | |||
"generate-api-docs-1": "cd scripts && node generate_api_docs_1.js", | |||
"generate-api-docs-3": "cd scripts && node generate_api_docs_3.js" |
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'd rename generate-api-docs
to generate-api-docs-2
@@ -0,0 +1,33 @@ | |||
// @ts-check |
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.
license
scripts/gen_and_summarize.js
Outdated
|
||
if (require.main === module) { | ||
main().catch((e) => { | ||
console.error(e ? e.stack || e.message || e: `Unknown error`); |
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 formatting here makes this a little hard to parse
@@ -0,0 +1,501 @@ | |||
/** |
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.
license
scripts/generate_api_docs_3.js
Outdated
const map = new Map(); | ||
/** @param {SimpleFeature} feature */ | ||
function index(feature, kind) { | ||
const filename = getFilename(feature) || '???'; |
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.
warn?
scripts/generate_api_docs_3.js
Outdated
} | ||
|
||
const hardcodedDescriptions = new Map([ | ||
['lib/elements/array-selector.js', ` |
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.
What's the way forward here? Can you open issues in Analyzer and Polymer to add these to the source?
scripts/generate_api_docs_3.js
Outdated
const [feature] = features; | ||
description = feature.summary; | ||
} | ||
description = description || hardcodedDescriptions.get(filename) || `TODO(write me): ${filename}`; |
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.
TODO or just warn?
Maybe these debug defaults should only be written with a flag on.
All comments addressed. Polymer/tools#241 is the path forward for file overviews, and instead of producing ??? or TODO in the final output, it throws instead. |
A few thoughts: Current file overview is a little confusing. Especially weird to have the tiny import line be the first thing on the page, followed by a giant headline for the element/class. This might look better once the Current format doesn't seem to document symbols that are re-exported. For example, the My first thought was that it was weird to have the element/class embedded directly in the module page , but I guess the vast majority of our modules only export one thing. The challenge here is that if a single module exported two elements, or an element and some functions, it would be virtually impossible to figure out that the other items were there unless you knew to look for them. I think this issue could be addressed with a quick-reference at the beginning of the file, so you have something like: File overview description import {} from '@polymer/polymer/polymer-element.js' Exports: PolymerElement, html, fooBar Where each item in the Exports list is a link to the corresponding item. But this might look weird if almost all of our files have only a single export. A TOC or some other navigation aid would serve the same purpose of orienting people in the file (in fact, it'd be even better for large elements/mixins with lots of props and methods). Maybe we can get someone with a design-y eye to look at it. As far as the URLs go, the current URL fragments assume that each module is only exporting a single class/element. For example, if you had a single module that exported simple-dialog and fancy-dialog, each with an my-dialogs#method-open Not sure if I am borrowing trouble here. But I'd hate to rewrite all of the links to the API docs and then change the URLs, so let's discuss this assumption. |
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.
Left feedback on the main PR page about navigation and URL structure. I think we should discuss how to handle these before we launch. I don't think either of them needs to block merging of this PR. But we shouldn't rewrite the links to the API docs until we've discussed URL structure.
Opened #2541 to track suggestions above. |
Fixes #2379
Needs Polymer/tools#220 in order to produce this output, though we could merge this before that lands if we needed to.