Update docs/admin.md (#46060)

Describe the blessing thing, and a few updates (mainly the pointer to
the new project board).
This commit is contained in:
Eli Barzilay 2020-07-14 08:00:29 -04:00 committed by GitHub
parent 164eefe416
commit 6741553611
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -4,7 +4,7 @@ Welcome! This is a resource for the person who is on call for Definitely Typed.
assigned to Definitely Typed duty, where they do a week on-call. You can see the [schedule here](http://aka.ms/DTRotation).
When on-call, your goal is to try keep on top of the many open PRs for Definitely Typed; they are categorized into
different sections inside the [Projects board](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/4) on this repo.
different sections inside the [Projects board](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/5) on this repo.
You should scan from left to right through the boards, and ideally try to start at the oldest PR in a section and work
your way through to the newest. There is a tool: [`focus-dt`](https://github.com/DefinitelyTyped/focus-dt) which can help with this.
@ -15,6 +15,7 @@ of how well the changes fit the rest of the type definitions and if it could bre
With the impact understood, your job is to delegate to the people who actually know how to review the code: users
of the library who have agreed to look after the types.
### Looking at a PR
For a [quick TLDR overview, you can read these slides](https://docs.google.com/presentation/d/1Q4xfZSY7d9yHhtxSyb-DE85fTXB38RyF3nnyVyvenwc/edit#slide=id.p).
@ -24,6 +25,31 @@ Some key concepts:
- Popular packages can break more installs, and will need more time and focus
- There are a handful of authors who have shipped a lot of high quality contributions who you can happily delegate to
##### "Blessing" a PR
If a PR looks kind of ok, but you don't want to submit an approving review (it can be mistaken as an expert approval),
then move the PR *away* from the `Needs Maintainer Review` and the bot will interpret that as an implicit blessing. (Do
this using the dropdown column in the "Projects" box on the right.) Like reviews, updates to the PR will void such a
blessing.
The column you move it to doesn't make any difference to the bot, it will move it to the right one if needed, but it
works best to move it to `Waiting for Code Reviews`.
> **Disclaimer:** It is currently impossible to get from/to information about column moves, so the bot ignores the
> column it was moved from. This means that it is impossible to cancel a blessing, but you can still submit a review if
> changes are needed.
##### Project board columns
- `Needs Maintainer Action`: PRs that cannot be dismissed with a blessing
- `Needs Maintainer Review`: Main maintainer queue column
- `Other`: Something went wrong, PRs can pop up there for a short time before the test results are in
- `Waiting for Author to Merge`: All good, and the owner can self-merge their PR
- `Needs Author Action`, `Recently Merged`, `Waiting for Code Reviews`: Self describing
##### Amending an existing Definitely Typed Package
An ideal PR to a DT package looks like:
@ -32,18 +58,18 @@ An ideal PR to a DT package looks like:
- 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
Most of the PRs are like this, in which case 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 which you should consider:
- Will the PR break existing code?
- This can sometimes be hard to decipher from the diff, e.g. an additional only PR may break superclasses of a class
- This can sometimes be hard to decipher from the diff, e.g., an addition-only PR may break superclasses of a class
- We try to make sure that types are a semver match on `major.min` for the library they represent
- 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
- Is it popular? (e.g. do you recognise it) if so, it should probably have two sign-offs
- Is it popular? (e.g., do you recognize it) if so, it should probably have two sign-offs
- 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
- The `tslint.json` does not have exceptions to the rules in it
@ -67,28 +93,28 @@ Constraints which you should consider:
```
`react-*` packages get a pass on this.
- Their `tsconfig.json` should never have `esModuleInterop: true`, which would hide the above issue.
Their `tsconfig.json` should never have `esModuleInterop: true`, which would hide the above issue.
##### New Definitely Typed 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.
### 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 Definitely Typed
- [DT projects](https://github.com/DefinitelyTyped/DefinitelyTyped/projects/5) — an automated board saying where open PRs live
- [dt-merge-bot](https://github.com/DefinitelyTyped/dt-mergebot/) — the bot which sets the labels on PRs, and update's project status
- [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 Definitely Typed
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
- [types-publisher](https://github.com/microsoft/DefinitelyTyped-tools/tree/master/packages/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
- [dts-gen](https://github.com/Microsoft/dts-gen) a tool for creating a DT folder automatically