Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
- Moved `readme` back to it's own property
- Stopped enforcing the stability banner in readme (belongs to another tool/layer)
- Added `Stability.Deprecated`, and enforced that deprecated packages use it
- Used the NodeJS stability index in documentation
  • Loading branch information
RomainMuller committed Jun 4, 2019
1 parent 9bb8422 commit c935c0f
Show file tree
Hide file tree
Showing 12 changed files with 67 additions and 75 deletions.
1 change: 0 additions & 1 deletion packages/jsii-calc/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# jsii Calculator
![API Stability: experimental](https://img.shields.io/badge/API%20Stability-experimental-important.svg)

This library is used to demonstrate and test the features of JSII

Expand Down
9 changes: 5 additions & 4 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@
},
"description": "A simple calcuator built on JSII.",
"docs": {
"remarks": "![API Stability: experimental](https://img.shields.io/badge/API%20Stability-experimental-important.svg)\n\nThis library is used to demonstrate and test the features of JSII\n\n## Sphinx\n\nThis file will be incorporated into the sphinx documentation.\n\nIf this file starts with an \"H1\" line (in our case `# jsii Calculator`), this\nheading will be used as the Sphinx topic name. Otherwise, the name of the module\n(`jsii-calc`) will be used instead.\n\n\n\n\n",
"stability": "experimental",
"summary": "# jsii Calculator"
"stability": "experimental"
},
"homepage": "https://github.com/awslabs/jsii.git",
"jsiiVersion": "0.11.0",
Expand All @@ -198,6 +196,9 @@
}
},
"name": "jsii-calc",
"readme": {
"markdown": "# jsii Calculator\n\nThis library is used to demonstrate and test the features of JSII\n\n## Sphinx\n\nThis file will be incorporated into the sphinx documentation.\n\nIf this file starts with an \"H1\" line (in our case `# jsii Calculator`), this\nheading will be used as the Sphinx topic name. Otherwise, the name of the module\n(`jsii-calc`) will be used instead.\n\n\n\n\n"
},
"repository": {
"type": "git",
"url": "https://github.com/awslabs/jsii.git"
Expand Down Expand Up @@ -6812,5 +6813,5 @@
}
},
"version": "0.11.0",
"fingerprint": "5ejpZLVHjfmn1dghKkB/85hdGJuSRGZtxSIH+SF+b6Y="
"fingerprint": "KKOrdvjcsF9ItTXwTs/c4WE7VmYP0adJh/N9hx3+USo="
}
11 changes: 2 additions & 9 deletions packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,15 +356,8 @@ class JavaGenerator extends Generator {
const packageInfoFile = this.toJavaFilePath(mod.name + '.package-info');
this.code.openFile(packageInfoFile);
this.code.line('/**');
if (mod.docs.summary || mod.docs.remarks) {
const mdown = new Array<string>();
if (mod.docs.summary) {
mdown.push(mod.docs.summary);
}
if (mod.docs.remarks) {
mdown.push(mod.docs.remarks);
}
for (const line of md2html(mdown.join('\n')).split('\n')) {
if (mod.readme) {
for (const line of md2html(mod.readme.markdown).split('\n')) {
this.code.line(` * ${line}`);
}
this.code.line(' *');
Expand Down
5 changes: 1 addition & 4 deletions packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1097,10 +1097,7 @@ class Package {
}

code.openFile("README.md");
const readme = this.metadata.docs && this.metadata.docs.summary
? `${this.metadata.docs.summary}\n${this.metadata.docs.remarks || ''}`
: '';
code.line(readme);
code.line(this.metadata.readme && this.metadata.readme.markdown);
code.closeFile("README.md");

// Strip " (build abcdef)" from the jsii version
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-pacmak/lib/targets/sphinx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -714,14 +714,14 @@ class SphinxDocsGenerator extends Generator {
* @returns header: the contents of the header (or undefined)
*/
private emitReadme(assm: spec.Assembly): { readmeFile?: string, readmeHeader?: string } {
if (!assm.docs || !assm.docs.summary) {
if (!assm.readme) {
return {
readmeFile: undefined,
readmeHeader: undefined
};
}

let lines = [assm.docs.summary, ...(assm.docs.remarks && assm.docs.remarks.split('\n') || [])];
let lines = assm.readme.markdown.split('\n');
let readmeHeader;

if (lines[0].startsWith('# ')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@
},
"description": "A simple calcuator built on JSII.",
"docs": {
"remarks": "![API Stability: experimental](https://img.shields.io/badge/API%20Stability-experimental-important.svg)\n\nThis library is used to demonstrate and test the features of JSII\n\n## Sphinx\n\nThis file will be incorporated into the sphinx documentation.\n\nIf this file starts with an \"H1\" line (in our case `# jsii Calculator`), this\nheading will be used as the Sphinx topic name. Otherwise, the name of the module\n(`jsii-calc`) will be used instead.\n\n\n\n\n",
"stability": "experimental",
"summary": "# jsii Calculator"
"stability": "experimental"
},
"homepage": "https://github.com/awslabs/jsii.git",
"jsiiVersion": "0.11.0",
Expand All @@ -198,6 +196,9 @@
}
},
"name": "jsii-calc",
"readme": {
"markdown": "# jsii Calculator\n\nThis library is used to demonstrate and test the features of JSII\n\n## Sphinx\n\nThis file will be incorporated into the sphinx documentation.\n\nIf this file starts with an \"H1\" line (in our case `# jsii Calculator`), this\nheading will be used as the Sphinx topic name. Otherwise, the name of the module\n(`jsii-calc`) will be used instead.\n\n\n\n\n"
},
"repository": {
"type": "git",
"url": "https://github.com/awslabs/jsii.git"
Expand Down Expand Up @@ -6812,5 +6813,5 @@
}
},
"version": "0.11.0",
"fingerprint": "5ejpZLVHjfmn1dghKkB/85hdGJuSRGZtxSIH+SF+b6Y="
"fingerprint": "KKOrdvjcsF9ItTXwTs/c4WE7VmYP0adJh/N9hx3+USo="
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/**
* <h1>jsii Calculator</h1>
* <p><img src="https://img.shields.io/badge/API%20Stability-experimental-important.svg" alt="API Stability: experimental" /></p>
* <p>This library is used to demonstrate and test the features of JSII</p>
* <h2>Sphinx</h2>
* <p>This file will be incorporated into the sphinx documentation.</p>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# jsii Calculator
![API Stability: experimental](https://img.shields.io/badge/API%20Stability-experimental-important.svg)

This library is used to demonstrate and test the features of JSII

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
![API Stability: experimental](https://img.shields.io/badge/API%20Stability-experimental-important.svg)

This library is used to demonstrate and test the features of JSII

Expand Down
25 changes: 22 additions & 3 deletions packages/jsii-spec/lib/spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ export interface Assembly extends Documentable {
* @default none
*/
types?: { [fqn: string]: Type };

/**
* The top-level readme document for this assembly (if any).
*
* @default none
*/
readme?: { markdown: string };
}

/**
Expand Down Expand Up @@ -327,15 +334,27 @@ export interface Docs {
}

/**
* API Stability levels.
* API Stability levels. These are modeled after the `node` stability index.
*
* @see https://nodejs.org/api/documentation.html#documentation_stability_index.
*/
export enum Stability {
/**
* Experimental APIs may change in breaking ways in a minor version update.
* The API may emit warnings. Backward compatibility is not guaranteed.
*/
Deprecated = 'deprecated',

/**
* This API is still under active development and subject to non-backward
* compatible changes or removal in any future version. Use of the API is
* not recommended in production environments. Experimental APIs are not
* subject to the Semantic Versioning model.
*/
Experimental = 'experimental',

/**
* Stable APIs may not change in breaking ways without a major version bump.
* This API is subject to the Semantic Versioning model and may not change
* in breaking ways in a subsequent minor or patch version.
*/
Stable = 'stable',
}
Expand Down
57 changes: 17 additions & 40 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,13 @@ export class Assembler implements Emitter {
ts.DiagnosticCategory.Suggestion,
'A "homepage" field should be specified in "package.json"');
}
const docs = await _loadReadme.call(this);
if (!docs || !docs.summary) {
const readme = await _loadReadme.call(this);
if (readme == null) {
this._diagnostic(null,
ts.DiagnosticCategory.Suggestion,
'There is no "README.md" file. It is recommended to have one.');
}

if (docs && docs.stability) {
const badge = _stabilityBadge(docs.stability);
if (!docs || !docs.remarks || docs.remarks.indexOf(badge) < 0) {
this._diagnostic(null,
ts.DiagnosticCategory.Error,
`The "README.md" file does not document the API stability (${docs.stability}). The following markdown badge is required:\n\t${badge}`);
}
}
const docs = _loadDocs.call(this);

this._types = {};
this._deferred = [];
Expand Down Expand Up @@ -125,6 +117,7 @@ export class Assembler implements Emitter {
targets: this.projectInfo.targets,
metadata: this.projectInfo.metadata,
docs,
readme,
jsiiVersion,
fingerprint: '<TBD>',
};
Expand All @@ -150,26 +143,25 @@ export class Assembler implements Emitter {
delete this._diagnostics;
}

async function _loadReadme(this: Assembler): Promise<spec.Docs | undefined> {
async function _loadReadme(this: Assembler) {
const readmePath = path.join(this.projectInfo.projectRoot, 'README.md');
if (!await fs.pathExists(readmePath)) {
return this.projectInfo.deprecated || this.projectInfo.stability
? {
deprecated: this.projectInfo.deprecated,
stability: this.projectInfo.stability
}
: undefined;
return undefined;
}
const [summary, ...remarks] = await literate.includeAndRenderExamples(
const markdown = await literate.includeAndRenderExamples(
await literate.loadFromFile(readmePath),
literate.fileSystemLoader(this.projectInfo.projectRoot)
);
return {
deprecated: this.projectInfo.deprecated,
stability: this.projectInfo.stability,
summary,
remarks: remarks.join('\n')
};
return { markdown: markdown.join('\n') };
}

function _loadDocs(this: Assembler): spec.Docs | undefined {
if (!this.projectInfo.stability && !this.projectInfo.deprecated) {
return undefined;
}
const deprecated = this.projectInfo.deprecated;
const stability = this.projectInfo.stability;
return { deprecated, stability };
}
}

Expand Down Expand Up @@ -1650,18 +1642,3 @@ const PROHIBITED_MEMBER_NAMES = ['equals', 'hashcode'];
function isProhibitedMemberName(name: string) {
return PROHIBITED_MEMBER_NAMES.includes(name.toLowerCase());
}

function _stabilityBadge(stability: spec.Stability) {
return `![API Stability: ${stability}](https://img.shields.io/badge/API%20Stability-${stability}-${_stabilityColor()}.svg)`;

function _stabilityColor() {
switch (stability) {
case spec.Stability.Stable:
return 'success';
case spec.Stability.Experimental:
return 'important';
default:
return 'critical';
}
}
}
18 changes: 13 additions & 5 deletions packages/jsii/lib/project-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export async function loadProjectInfo(projectRoot: string, { fixPeerDependencies
name: _required(pkg.name, 'The "package.json" file must specify the "name" attribute'),
version: _required(pkg.version, 'The "package.json" file must specify the "version" attribute'),
deprecated: pkg.deprecated,
stability: pkg.stability && _validateStability(pkg.stability),
stability: _validateStability(pkg.stability, pkg.deprecated),
author: _toPerson(_required(pkg.author, 'The "package.json" file must specify the "author" attribute'), 'author'),
repository: {
url: _required(pkg.repository.url, 'The "package.json" file must specify the "repository.url" attribute'),
Expand Down Expand Up @@ -233,9 +233,17 @@ function _validateVersionFormat(format: string): 'short' | 'full' {
return format;
}

function _validateStability(stability: string): spec.Stability {
if (Object.values(spec.Stability).indexOf(stability) !== -1) {
return stability as spec.Stability;
function _validateStability(stability: string | undefined, deprecated: string | undefined): spec.Stability | undefined {
if (!stability && deprecated) {
stability = spec.Stability.Deprecated;
} else if (deprecated && stability !== spec.Stability.Deprecated) {
throw new Error(`Package is deprecated (${deprecated}), but it's stability is ${stability} and not ${spec.Stability.Deprecated}`);
}
throw new Error(`Invalid stability "${stability}", it must be one of ${Object.values(spec.Stability).join(', ')}`);
if (!stability) {
return undefined;
}
if (Object.values(spec.Stability).indexOf(stability) === -1) {
throw new Error(`Invalid stability "${stability}", it must be one of ${Object.values(spec.Stability).join(', ')}`);
}
return stability as spec.Stability;
}

0 comments on commit c935c0f

Please sign in to comment.