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

Jump to bottom

Adds some admin docs #41063

Merged
merged 5 commits into from Dec 24, 2019
Merged

Adds some admin docs #41063

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85bfc99
Adds some admin docs
orta Dec 16, 2019
57c1638
Update admin manual
orta Dec 16, 2019
3e999fe
Update docs/admin.md
orta Dec 17, 2019
504a1b7
More docs on admin
orta Dec 18, 2019
fb9e2e9
Merge branch 'admin_docs' of https://github.com/DefinitelyTyped/Defin…
orta Dec 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 deletions README.md
View file
Open in desktop
Expand Up @@ -2,10 +2,10 @@

> The repository for *high quality* TypeScript type definitions.
Also see the [definitelytyped.org](http://definitelytyped.org) website, although information in this README is more up-to-date.

*You can also read this README in [Spanish](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/README.es.md), [Korean](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/README.ko.md), [Russian](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/README.ru.md), and [Chinese](https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/README.cn.md)!*

*Link to [Admin manual](./docs/admin.md)*

## Table of Contents

* [Current status](#current-status)
Expand Down Expand Up @@ -84,13 +84,12 @@ For example, if you run `npm dist-tags @types/react`, you'll see the following t

### Typescript 1.8 and earlier

* [Typings](https://github.com/typings/typings)
* Manually download from the `master` branch of this repository and place them in your project
* ~~[Typings](https://github.com/typings/typings)~~ (use preferred alternatives, typings is deprecated)
* ~~[NuGet](http://nuget.org/packages?q=DefinitelyTyped)~~ (use preferred alternatives, nuget DT type publishing has been turned off)
* Manually download from the `master` branch of this repository

You may need to add manual [references](http://www.typescriptlang.org/docs/handbook/triple-slash-directives.html).


## How can I contribute?

Definitely Typed only works because of contributions by users like you!
Expand Down Expand Up @@ -183,7 +182,7 @@ For a good example package, see [base64-js](https://github.com/DefinitelyTyped/D
#### Common mistakes

* First, follow advice from the [handbook](http://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html).
* Formatting: Use 4 spaces. For new code, this is enforced by Prettier.
* Formatting: Use 4 spaces. Prettier is set up on this repo, so you can run `npm run prettier -- --write path/to/package`.
orta marked this conversation as resolved.
Show resolved Hide resolved
* `function sum(nums: number[]): number`: Use `ReadonlyArray` if a function does not write to its parameters.
* `interface Foo { new(): Foo; }`:
This defines a type of objects that are new-able. You probably want `declare class Foo { constructor(); }`.
Expand Down
59 changes: 59 additions & 0 deletions docs/admin.md
View file
Open in desktop
@@ -0,0 +1,59 @@
## Admin Manual for Definitely Typed

Welcome! This is a resource for the person who is on call for DefinitelyTyped. The TypeScript team always has someone
assigned to DefinitelyTyped duty, where they do a week on-call. You can see the [schedule here](http://aka.ms/DTRotation).

Copy link
Contributor

Choose a reason for hiding this comment

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

These two paragraphs are a good start, although eventually I'd like to have a bit more about the philosophy — that this isn't a code review in the usual sense, but an assessment of category and severity, followed by delegating to the people who actually know how to review the code. Since the bot tags the PRs with category and owners, it's mainly an exercise in co-ordinating between humans, and filling in any places the bot currently misses.

Also, something about how the sliding scale of package popularity affects reviews. The maintainer should review changes to popular packages, and very popular packages — those with a name you recognise, roughly — should have signoffs from at least two maintainers. Conversely, it's fine to overlook problems with changes to unpopular packages; in many cases only the author is using them anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree - I've expanded it!

When on-call, your goal is to try keep on top of the many open PRs for DefinitelyTyped, they are categorized into
orta marked this conversation as resolved.
Show resolved Hide resolved
different sections inside the [Projects board](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/4) on this repo.

You should scan from left to right through the boards, and ideally try to start at the latest PR and work your way
orta marked this conversation as resolved.
Show resolved Hide resolved
through to the newest.

There is a tool: [`focus-dt`](https://github.com/DefinitelyTyped/focus-dt) which can help with this.

### Looking at a PR

Some useful heuristics for looking at a PR:

##### New DefinitelyTyped Package

- Is the author also a maintainer of the library? If so, they could use [`--declaration` and `--allowJs`](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#--declaration-and---allowjs) in TypeScript to generate their types.

Copy link
Contributor

Choose a reason for hiding this comment

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

at some point we should have the content of 'oh no modules' in here, plus links to the handbook. for now can you add a list like

  • no exceptions in tslint.json
  • strict: true or equivalent four (five?) settings in tsconfig.json
  • no esModuleInterop: true, no matter how tempting
  • make sure export default is actually default in the source; react-* packages get a pass on this.

Also maybe link to my slide deck: https://docs.google.com/presentation/d/1Q4xfZSY7d9yHhtxSyb-DE85fTXB38RyF3nnyVyvenwc/edit?usp=sharing


##### Amending an existing DefinitelyTyped Package

An ideal PR to a DT package looks like:

- A link in the PR description to the API being added
- Only additions to the existing types
- Test code which covers the existing use case

Most of the PRs are like this, in which case then a review for that PR should be pretty quick. Look through the code
changes, then see if there are comments asking for the merge to be delayed. If not, then you're good to merge. You
can then leave a thanks comment and hit the merge button.

Constraints on merging:

- Will the PR break existing code? If so, check that there's a semver bump
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hard in two ways:

  1. Anything that's not a strict addition is likely to break code; and some additions break code, for example if they're to a class with subclasses.
  2. DT packages are supposed to match the source's major.minor, so to bump semver, breaking changes can only ship when updating the types to a new major version.

The result is that so far we've told people that the Right Way to consume DT packages is to pin them to an exact major.minor.patch. That's not in the README and it's not common practise, but I think those are the things to change.

- The PR has merge conflicts, you can try edit the PR using the GitHub UI if it's a trivial change, then come back tomorrow
- The PR has no tests, possibly the package on DT hasn't got tests set up. You can decide if that's a blocker or not depending on how likely the code is going to break existing code


### Useful Repos and Links

The process of handling PRs:

- [DT projects](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/4) - an automated board saying where open PRs live
- [dt-merge-bot](https://github.com/RyanCavanaugh/dt-mergebot/) - the bot which sets the labels on PRs, and update's project status
- [dt-merge-bot graphql](https://github.com/RyanCavanaugh/dt-mergebot/tree/use-graphql) - the WIP v2 of the bot to automate the labels/projects
- [focus-dt](https://github.com/DefinitelyTyped/focus-dt) - a tool which controls chrome to load up the next PR to review, so you can focus
- [dtslint](https://github.com/microsoft/dtslint) - the CLI tool used to validate PRs on DefinitelyTyped

The process of deploying changes:

- [types-publisher](https://github.com/microsoft/types-publisher) - when a PR is merged, types-publisher moves the contents to NPM/GitHub Package Repository
- [CI](https://dev.azure.com/definitelytyped/DefinitelyTyped/_build) - the build pipelines on Azure which does most of the work

Recommendations we make:

- [dts-gen](https://github.com/Microsoft/dts-gen) - a tool for creating a DT folder automatically