From 5de077ab6558129d57e1212e0896caa5d5318eb1 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sat, 9 May 2020 09:05:23 -0400 Subject: Move style guide to docs dir (#5174) --- .github/PULL_REQUEST_TEMPLATE.md | 3 +- docs/contributing.md | 4 +- docs/contributing/style_guide.md | 316 +++++++++++++++++++++++++++++++++++++++ docs/toc.json | 1 + std/style_guide.md | 316 --------------------------------------- 5 files changed, 321 insertions(+), 319 deletions(-) create mode 100644 docs/contributing/style_guide.md delete mode 100644 std/style_guide.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index d988bb02a..3147bfbc4 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,3 +1,4 @@ diff --git a/docs/contributing.md b/docs/contributing.md index c650af79d..df2d1de24 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1,6 +1,6 @@ # Contributing -- Read the [style guide](style_guide.md). +- Read the [style guide](contributing/style_guide.md). - Please don't make [the benchmarks](https://deno.land/benchmarks.html) worse. @@ -16,7 +16,7 @@ ## Development Instructions on how to build from source can be found -[here](./building-from-source). +[here](./contributing/building_from_source.md). ## Submitting a Pull Request diff --git a/docs/contributing/style_guide.md b/docs/contributing/style_guide.md new file mode 100644 index 000000000..be953136f --- /dev/null +++ b/docs/contributing/style_guide.md @@ -0,0 +1,316 @@ +# Deno Style Guide + +## Table of Contents + +## Copyright Headers + +Most modules in the repository should have the following copyright header: + +```ts +// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. +``` + +If the code originates elsewhere, ensure that the file has the proper copyright +headers. We only allow MIT, BSD, and Apache licensed code. + +## Use underscores, not dashes in filenames. + +Example: Use `file_server.ts` instead of `file-server.ts`. + +## Add tests for new features. + +Each module should contain or be accompanied by tests for its public +functionality. + +## TODO Comments + +TODO comments should usually include an issue or the author's github username in +parentheses. Example: + +```ts +// TODO(ry): Add tests. +// TODO(#123): Support Windows. +// FIXME(#349): Sometimes panics. +``` + +## Meta-programming is discouraged. Including the use of Proxy. + +Be explicit even when it means more code. + +There are some situations where it may make sense to use such techniques, but in +the vast majority of cases it does not. + +## Rust + +Follow Rust conventions and be consistent with existing code. + +## Typescript + +The TypeScript portions of the codebase include `cli/js` for the built-ins and +the standard library `std`. + +### Use TypeScript instead of JavaScript. + +### Use the term "module" instead of "library" or "package". + +For clarity and consistency avoid the terms "library" and "package". Instead use +"module" to refer to a single JS or TS file and also to refer to a directory of +TS/JS code. + +### Do not use the filename `index.ts`/`index.js`. + +Deno does not treat "index.js" or "index.ts" in a special way. By using these +filenames, it suggests that they can be left out of the module specifier when +they cannot. This is confusing. + +If a directory of code needs a default entry point, use the filename `mod.ts`. +The filename `mod.ts` follows Rust’s convention, is shorter than `index.ts`, and +doesn’t come with any preconceived notions about how it might work. + +### Exported functions: max 2 args, put the rest into an options object. + +When designing function interfaces, stick to the following rules. + +1. A function that is part of the public API takes 0-2 required arguments, plus + (if necessary) an options object (so max 3 total). + +2. Optional parameters should generally go into the options object. + + An optional parameter that's not in an options object might be acceptable if + there is only one, and it seems inconceivable that we would add more optional + parameters in the future. + + + + +3. The 'options' argument is the only argument that is a regular 'Object'. + + Other arguments can be objects, but they must be distinguishable from a + 'plain' Object runtime, by having either: + + - a distinguishing prototype (e.g. `Array`, `Map`, `Date`, `class MyThing`) + - a well-known symbol property (e.g. an iterable with `Symbol.iterator`). + + This allows the API to evolve in a backwards compatible way, even when the + position of the options object changes. + + + +```ts +// BAD: optional parameters not part of options object. (#2) +export function resolve( + hostname: string, + family?: "ipv4" | "ipv6", + timeout?: number +): IPAddress[] {} + +// GOOD. +export interface ResolveOptions { + family?: "ipv4" | "ipv6"; + timeout?: number; +} +export function resolve( + hostname: string, + options: ResolveOptions = {} +): IPAddress[] {} +``` + +```ts +export interface Environment { + [key: string]: string; +} + +// BAD: `env` could be a regular Object and is therefore indistinguishable +// from an options object. (#3) +export function runShellWithEnv(cmdline: string, env: Environment): string {} + +// GOOD. +export interface RunShellOptions { + env: Environment; +} +export function runShellWithEnv( + cmdline: string, + options: RunShellOptions +): string {} +``` + +```ts +// BAD: more than 3 arguments (#1), multiple optional parameters (#2). +export function renameSync( + oldname: string, + newname: string, + replaceExisting?: boolean, + followLinks?: boolean +) {} + +// GOOD. +interface RenameOptions { + replaceExisting?: boolean; + followLinks?: boolean; +} +export function renameSync( + oldname: string, + newname: string, + options: RenameOptions = {} +) {} +``` + +```ts +// BAD: too many arguments. (#1) +export function pwrite( + fd: number, + buffer: TypedArray, + offset: number, + length: number, + position: number +) {} + +// BETTER. +export interface PWrite { + fd: number; + buffer: TypedArray; + offset: number; + length: number; + position: number; +} +export function pwrite(options: PWrite) {} +``` + +### Minimize dependencies; do not make circular imports. + +Although `cli/js` and `std` have no external dependencies, we must still be +careful to keep internal dependencies simple and manageable. In particular, be +careful not to introduce circular imports. + +### If a filename starts with an underscore: `_foo.ts`, do not link to it. + +Sometimes there maybe situations where an internal module is necessary but its +API is not meant to be stable or linked to. In this case prefix it with an +underscore. By convention, only files in its own directory should import it. + +### Use JSDoc for exported symbols. + +We strive for complete documentation. Every exported symbol ideally should have +a documentation line. + +If possible, use a single line for the JS Doc. Example: + +```ts +/** foo does bar. */ +export function foo() { + // ... +} +``` + +It is important that documentation is easily human readable, but there is also a +need to provide additional styling information to ensure generated documentation +is more rich text. Therefore JSDoc should generally follow markdown markup to +enrich the text. + +While markdown supports HTML tags, it is forbidden in JSDoc blocks. + +Code string literals should be braced with the back-tick (\`) instead of quotes. +For example: + +```ts +/** Import something from the `deno` module. */ +``` + +Do not document function arguments unless they are non-obvious of their intent +(though if they are non-obvious intent, the API should be considered anyways). +Therefore `@param` should generally not be used. If `@param` is used, it should +not include the `type` as TypeScript is already strongly typed. + +```ts +/** + * Function with non obvious param. + * @param foo Description of non obvious parameter. + */ +``` + +Vertical spacing should be minimized whenever possible. Therefore single line +comments should be written as: + +```ts +/** This is a good single line JSDoc. */ +``` + +And not + +```ts +/** + * This is a bad single line JSDoc. + */ +``` + +Code examples should not utilise the triple-back tick (\`\`\`) notation or tags. +They should just be marked by indentation, which requires a break before the +block and 6 additional spaces for each line of the example. This is 4 more than +the first column of the comment. For example: + +```ts +/** A straight forward comment and an example: + * + * import { foo } from "deno"; + * foo("bar"); + */ +``` + +Code examples should not contain additional comments. It is already inside a +comment. If it needs further comments it is not a good example. + +### Each module should come with a test module. + +Every module with public functionality `foo.ts` should come with a test module +`foo_test.ts`. A test for a `cli/js` module should go in `cli/js/tests` due to +their different contexts, otherwise it should just be a sibling to the tested +module. + +### Unit Tests should be explicit. + +For a better understanding of the tests, function should be correctly named as +its prompted throughout the test command. Like: + +``` +test myTestFunction ... ok +``` + +Example of test: + +```ts +import { assertEquals } from "https://deno.land/std@v0.11/testing/asserts.ts"; +import { foo } from "./mod.ts"; + +Deno.test("myTestFunction" function() { + assertEquals(foo(), { bar: "bar" }); +}); +``` + +### Top level functions should not use arrow syntax. + +Top level functions should use the `function` keyword. Arrow syntax should be +limited to closures. + +Bad + +```ts +export const foo = (): string => { + return "bar"; +}; +``` + +Good + +```ts +export function foo(): string { + return "bar"; +} +``` + +### `std` + +#### Do not depend on external code. + +`https://deno.land/std/` is intended to be baseline functionality that all Deno +programs can rely on. We want to guarantee to users that this code does not +include potentially unreviewed third party code. diff --git a/docs/toc.json b/docs/toc.json index 7956a402a..f87b1c0a5 100644 --- a/docs/toc.json +++ b/docs/toc.json @@ -65,6 +65,7 @@ "children": { "building_from_source": "Building from source", "development_tools": "Development tools", + "style_guide": "Style guide", "architecture": "Architecture" } }, diff --git a/std/style_guide.md b/std/style_guide.md deleted file mode 100644 index be953136f..000000000 --- a/std/style_guide.md +++ /dev/null @@ -1,316 +0,0 @@ -# Deno Style Guide - -## Table of Contents - -## Copyright Headers - -Most modules in the repository should have the following copyright header: - -```ts -// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -``` - -If the code originates elsewhere, ensure that the file has the proper copyright -headers. We only allow MIT, BSD, and Apache licensed code. - -## Use underscores, not dashes in filenames. - -Example: Use `file_server.ts` instead of `file-server.ts`. - -## Add tests for new features. - -Each module should contain or be accompanied by tests for its public -functionality. - -## TODO Comments - -TODO comments should usually include an issue or the author's github username in -parentheses. Example: - -```ts -// TODO(ry): Add tests. -// TODO(#123): Support Windows. -// FIXME(#349): Sometimes panics. -``` - -## Meta-programming is discouraged. Including the use of Proxy. - -Be explicit even when it means more code. - -There are some situations where it may make sense to use such techniques, but in -the vast majority of cases it does not. - -## Rust - -Follow Rust conventions and be consistent with existing code. - -## Typescript - -The TypeScript portions of the codebase include `cli/js` for the built-ins and -the standard library `std`. - -### Use TypeScript instead of JavaScript. - -### Use the term "module" instead of "library" or "package". - -For clarity and consistency avoid the terms "library" and "package". Instead use -"module" to refer to a single JS or TS file and also to refer to a directory of -TS/JS code. - -### Do not use the filename `index.ts`/`index.js`. - -Deno does not treat "index.js" or "index.ts" in a special way. By using these -filenames, it suggests that they can be left out of the module specifier when -they cannot. This is confusing. - -If a directory of code needs a default entry point, use the filename `mod.ts`. -The filename `mod.ts` follows Rust’s convention, is shorter than `index.ts`, and -doesn’t come with any preconceived notions about how it might work. - -### Exported functions: max 2 args, put the rest into an options object. - -When designing function interfaces, stick to the following rules. - -1. A function that is part of the public API takes 0-2 required arguments, plus - (if necessary) an options object (so max 3 total). - -2. Optional parameters should generally go into the options object. - - An optional parameter that's not in an options object might be acceptable if - there is only one, and it seems inconceivable that we would add more optional - parameters in the future. - - - - -3. The 'options' argument is the only argument that is a regular 'Object'. - - Other arguments can be objects, but they must be distinguishable from a - 'plain' Object runtime, by having either: - - - a distinguishing prototype (e.g. `Array`, `Map`, `Date`, `class MyThing`) - - a well-known symbol property (e.g. an iterable with `Symbol.iterator`). - - This allows the API to evolve in a backwards compatible way, even when the - position of the options object changes. - - - -```ts -// BAD: optional parameters not part of options object. (#2) -export function resolve( - hostname: string, - family?: "ipv4" | "ipv6", - timeout?: number -): IPAddress[] {} - -// GOOD. -export interface ResolveOptions { - family?: "ipv4" | "ipv6"; - timeout?: number; -} -export function resolve( - hostname: string, - options: ResolveOptions = {} -): IPAddress[] {} -``` - -```ts -export interface Environment { - [key: string]: string; -} - -// BAD: `env` could be a regular Object and is therefore indistinguishable -// from an options object. (#3) -export function runShellWithEnv(cmdline: string, env: Environment): string {} - -// GOOD. -export interface RunShellOptions { - env: Environment; -} -export function runShellWithEnv( - cmdline: string, - options: RunShellOptions -): string {} -``` - -```ts -// BAD: more than 3 arguments (#1), multiple optional parameters (#2). -export function renameSync( - oldname: string, - newname: string, - replaceExisting?: boolean, - followLinks?: boolean -) {} - -// GOOD. -interface RenameOptions { - replaceExisting?: boolean; - followLinks?: boolean; -} -export function renameSync( - oldname: string, - newname: string, - options: RenameOptions = {} -) {} -``` - -```ts -// BAD: too many arguments. (#1) -export function pwrite( - fd: number, - buffer: TypedArray, - offset: number, - length: number, - position: number -) {} - -// BETTER. -export interface PWrite { - fd: number; - buffer: TypedArray; - offset: number; - length: number; - position: number; -} -export function pwrite(options: PWrite) {} -``` - -### Minimize dependencies; do not make circular imports. - -Although `cli/js` and `std` have no external dependencies, we must still be -careful to keep internal dependencies simple and manageable. In particular, be -careful not to introduce circular imports. - -### If a filename starts with an underscore: `_foo.ts`, do not link to it. - -Sometimes there maybe situations where an internal module is necessary but its -API is not meant to be stable or linked to. In this case prefix it with an -underscore. By convention, only files in its own directory should import it. - -### Use JSDoc for exported symbols. - -We strive for complete documentation. Every exported symbol ideally should have -a documentation line. - -If possible, use a single line for the JS Doc. Example: - -```ts -/** foo does bar. */ -export function foo() { - // ... -} -``` - -It is important that documentation is easily human readable, but there is also a -need to provide additional styling information to ensure generated documentation -is more rich text. Therefore JSDoc should generally follow markdown markup to -enrich the text. - -While markdown supports HTML tags, it is forbidden in JSDoc blocks. - -Code string literals should be braced with the back-tick (\`) instead of quotes. -For example: - -```ts -/** Import something from the `deno` module. */ -``` - -Do not document function arguments unless they are non-obvious of their intent -(though if they are non-obvious intent, the API should be considered anyways). -Therefore `@param` should generally not be used. If `@param` is used, it should -not include the `type` as TypeScript is already strongly typed. - -```ts -/** - * Function with non obvious param. - * @param foo Description of non obvious parameter. - */ -``` - -Vertical spacing should be minimized whenever possible. Therefore single line -comments should be written as: - -```ts -/** This is a good single line JSDoc. */ -``` - -And not - -```ts -/** - * This is a bad single line JSDoc. - */ -``` - -Code examples should not utilise the triple-back tick (\`\`\`) notation or tags. -They should just be marked by indentation, which requires a break before the -block and 6 additional spaces for each line of the example. This is 4 more than -the first column of the comment. For example: - -```ts -/** A straight forward comment and an example: - * - * import { foo } from "deno"; - * foo("bar"); - */ -``` - -Code examples should not contain additional comments. It is already inside a -comment. If it needs further comments it is not a good example. - -### Each module should come with a test module. - -Every module with public functionality `foo.ts` should come with a test module -`foo_test.ts`. A test for a `cli/js` module should go in `cli/js/tests` due to -their different contexts, otherwise it should just be a sibling to the tested -module. - -### Unit Tests should be explicit. - -For a better understanding of the tests, function should be correctly named as -its prompted throughout the test command. Like: - -``` -test myTestFunction ... ok -``` - -Example of test: - -```ts -import { assertEquals } from "https://deno.land/std@v0.11/testing/asserts.ts"; -import { foo } from "./mod.ts"; - -Deno.test("myTestFunction" function() { - assertEquals(foo(), { bar: "bar" }); -}); -``` - -### Top level functions should not use arrow syntax. - -Top level functions should use the `function` keyword. Arrow syntax should be -limited to closures. - -Bad - -```ts -export const foo = (): string => { - return "bar"; -}; -``` - -Good - -```ts -export function foo(): string { - return "bar"; -} -``` - -### `std` - -#### Do not depend on external code. - -`https://deno.land/std/` is intended to be baseline functionality that all Deno -programs can rely on. We want to guarantee to users that this code does not -include potentially unreviewed third party code. -- cgit v1.2.3