|
| 1 | +## Admin Manual for Definitely Typed |
| 2 | + |
| 3 | +Welcome! This is a resource for the person who is on call for DefinitelyTyped. The TypeScript team always has someone |
| 4 | +assigned to DefinitelyTyped duty, where they do a week on-call. You can see the [schedule here](http://aka.ms/DTRotation). |
| 5 | + |
| 6 | +When on-call, your goal is to try keep on top of the many open PRs for DefinitelyTyped; they are categorized into |
| 7 | +different sections inside the [Projects board](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/4) on this repo. |
| 8 | + |
| 9 | +You should scan from left to right through the boards, and ideally try to start at the oldest PR in a section and work |
| 10 | +your way through to the newest. There is a tool: [`focus-dt`](https://github.com/DefinitelyTyped/focus-dt) which can help with this. |
| 11 | + |
| 12 | +Our goals are to provide a stable, high quality ecosystem of type definitions and that requires the compiler team |
| 13 | +making sure that a PR does not break the ecosystem. This work isn't a code review in the usual sense, but an assessment |
| 14 | +of how well the changes fit the rest of the type definitions and if it could break builds for our users or our systems. |
| 15 | +With the impact understood, your job is to delegate to the people who actually know how to review the code: users |
| 16 | +of the library who have agreed to look after the types. |
| 17 | + |
| 18 | +### Looking at a PR |
| 19 | + |
| 20 | +For a [quick TLDR overview, you can read these slides](https://docs.google.com/presentation/d/1Q4xfZSY7d9yHhtxSyb-DE85fTXB38RyF3nnyVyvenwc/edit#slide=id.p). |
| 21 | + |
| 22 | +Some key concepts: |
| 23 | + |
| 24 | +- Popular packages can break more installs, and will need more time and focus |
| 25 | +- There are a handful of authors who have shipped a lot of high quality contributions who you can happily delegate to |
| 26 | + |
| 27 | +##### Amending an existing DefinitelyTyped Package |
| 28 | + |
| 29 | +An ideal PR to a DT package looks like: |
| 30 | + |
| 31 | +- A link in the PR description to the API being added |
| 32 | +- Only additions to the existing types |
| 33 | +- Test code which covers the existing use case |
| 34 | + |
| 35 | +Most of the PRs are like this, in which case then a review for that PR should be pretty quick. Look through the code |
| 36 | +changes, then see if there are comments asking for the merge to be delayed. If not, then you're good to merge. You |
| 37 | +can then leave a thanks comment and hit the merge button. |
| 38 | + |
| 39 | +Constraints which you should consider: |
| 40 | + |
| 41 | +- Will the PR break existing code? |
| 42 | + - This can sometimes be hard to decipher from the diff, e.g. an additional only PR may break superclasses of a class |
| 43 | + - We try to make sure that types are a semver match on `major.min` for the library they represent |
| 44 | + - A build breaking change therefore can be a trade-off where you have to talk with the library maintainer, and get their sign-offs that it is worth it |
| 45 | + |
| 46 | +- Is it popular? (e.g. do you recognise it) if so, it should probably have two sign-offs |
| 47 | +- 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 |
| 48 | +- 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 |
| 49 | +- The `tslint.json` does not have exceptions to the rules in it |
| 50 | +- Strict mode is `strict: true` or equivalent four (five?) settings in tsconfig.json |
| 51 | +- Make sure export default is actually default in the source. |
| 52 | + |
| 53 | + ```ts |
| 54 | + // Cannot have `export default` in the dts |
| 55 | + modele.exports = { |
| 56 | + thing: () => "hello world" |
| 57 | + } |
| 58 | + ``` |
| 59 | + |
| 60 | + vs |
| 61 | + |
| 62 | + ```ts |
| 63 | + // Can import via `export default` |
| 64 | + modele.exports.default = { |
| 65 | + thing: () => "hello world" |
| 66 | + } |
| 67 | + ``` |
| 68 | + |
| 69 | + `react-*` packages get a pass on this. |
| 70 | +- Their `tsconfig.json` should never have `esModuleInterop: true`, which would hide the above issue. |
| 71 | + |
| 72 | + |
| 73 | +##### New DefinitelyTyped Package |
| 74 | + |
| 75 | +- 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. |
| 76 | + |
| 77 | +### Useful Repos and Links |
| 78 | + |
| 79 | +The process of handling PRs: |
| 80 | + |
| 81 | +- [DT projects](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/4) - an automated board saying where open PRs live |
| 82 | +- [dt-merge-bot](https://github.com/RyanCavanaugh/dt-mergebot/) - the bot which sets the labels on PRs, and update's project status |
| 83 | +- [dt-merge-bot graphql](https://github.com/RyanCavanaugh/dt-mergebot/tree/use-graphql) - the WIP v2 of the bot to automate the labels/projects |
| 84 | +- [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 |
| 85 | +- [dtslint](https://github.com/microsoft/dtslint) - the CLI tool used to validate PRs on DefinitelyTyped |
| 86 | + |
| 87 | +The process of deploying changes: |
| 88 | + |
| 89 | +- [types-publisher](https://github.com/microsoft/types-publisher) - when a PR is merged, types-publisher moves the contents to NPM/GitHub Package Repository |
| 90 | +- [CI](https://dev.azure.com/definitelytyped/DefinitelyTyped/_build) - the build pipelines on Azure which does most of the work |
| 91 | + |
| 92 | +Recommendations we make: |
| 93 | + |
| 94 | +- [dts-gen](https://github.com/Microsoft/dts-gen) - a tool for creating a DT folder automatically |
0 commit comments