-
Notifications
You must be signed in to change notification settings - Fork 26.9k
build(docs-infra): fix CLI command github links #30889
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,22 +26,59 @@ module.exports = function cliCommandFileReader(log) { | |
| docType: 'cli-command', | ||
| id: `cli-${doc.name}`, | ||
| commandAliases: doc.aliases || [], | ||
| aliases: computeAliases(doc), | ||
| path, | ||
| aliases: computeAliases(doc), path, | ||
| outputPath: `${path}.json`, | ||
| breadCrumbs: [ | ||
| { text: 'CLI', path: 'cli' }, | ||
| { text: name, path }, | ||
| {text: 'CLI', path: 'cli'}, | ||
| {text: name, path}, | ||
| ] | ||
| }); | ||
| if (doc.longDescription) { | ||
| doc.longDescriptionDoc = createLongDescriptionDoc(fileInfo); | ||
| } | ||
| return [result]; | ||
| } catch (e) { | ||
| log.warn(`Failed to read cli command file: "${fileInfo.relativePath}" - ${e.message}`); | ||
| } | ||
| } | ||
| }; | ||
| }; | ||
| function computeAliases(doc) { | ||
| return [doc.name].concat(doc.aliases || []).map(alias => `cli-${alias}`); | ||
| } | ||
|
|
||
| /** | ||
| * Synthesize a doc for the CLI command long description, which is used to generate links | ||
| * for viewing and editing the long description in GitHub. | ||
| * | ||
| * The long description is stored in a markdown file that is referenced from the original | ||
| * schema file for the command, via the `$longDescription` field. The field is a relative path | ||
| * to the markdown file from the schema file. | ||
| * | ||
| * This function tries to retrieve that original schema based on the file path of the help JSON | ||
| * file, which was passed to the `cliCommandFileReader.getDocs()` method. | ||
| */ | ||
| function createLongDescriptionDoc(fileInfo) { | ||
| try { | ||
| const path = require('canonical-path'); | ||
| const fs = require('fs'); | ||
| const json5 = require('json5'); | ||
|
|
||
| function computeAliases(doc) { | ||
| return [doc.name].concat(doc.aliases || []).map(alias => `cli-${alias}`); | ||
| } | ||
| const schemaJsonPath = path.resolve(fileInfo.basePath, '../commands', fileInfo.relativePath); | ||
| const schemaJson = fs.readFileSync(schemaJsonPath); | ||
| const schema = json5.parse(schemaJson); | ||
| if (schema.$longDescription) { | ||
| return { | ||
| docType: 'content', | ||
| startingLine: 0, | ||
| fileInfo: { | ||
| realProjectRelativePath: | ||
| path.join(path.dirname(fileInfo.realProjectRelativePath), schema.$longDescription) | ||
| } | ||
| }; | ||
| } | ||
| } catch (e) { | ||
| log.warn('Unable to read CLI long description file info', e, fileInfo); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not fail hard here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided that not having a link for the description would be better than failing the build altogether. But perhaps you are right that we should fail so that it picks up on a breaking change to the layout of the CLI project.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a precedent for this further up the file in this
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The more hard fails the better 🎉 |
||
| return undefined; | ||
| } | ||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "$longDescription": "./add-long.md" | ||
| } |
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.
Unfortunately, I don't think this is enough 😁
It generally works on branches, but doesn't seem to work on master. That is because
cli-builds#masterdoesn't seem to be always be updated. For example, currently cli-builds#master > package.json >version is 8.0.0-beta.18+168.6ec0991, while cli-builds#8.0.x > package.json > version is 8.0.0+13.958d4cc 😞Ideally, it should be fixed in the cli 😁
Or maybe we could always use
master, when onangular/angular#master?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 would we actually get the
masterbuild fromcli-builds?Don't we only install the version given by the build SHA. E.g.
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.
Right. But that cli-builds SHA can come from
cli-builds#masterorcli-build#<some-branch>.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 think that this needs fixing in CLI: angular/angular-cli#14692