Codify in checklist form the steps to take while reviewing a new image

[tianon]

This is a starting point to try and put these check points into something somewhat more formal such that we're less likely to forget a step.

cc @yosifkit @psftw @md5

[tianon]

Here's an example of how I'd envision adding this to the top of a PR, using #1201 (our oldest current proposal) as the guinea pig:

Checklist for Review

• associated with or contacted upstream?
  • Adding F# language #1201 (comment)
• does it fit into one of the common categories? ("service", "language stack", "base distribution")
• is it reasonably popular, or does it solve a particular use case well?
• does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • Add F# to docs docs#583
• review Dockerfiles for best practices and cache gotchas/improvements (ala the official review guidelines)
• existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
  • Adding F# language #1201 (comment)
• if FROM scratch, tarballs only exist in a single commit within the associated history?

[tianon]

Wondering if a final bullet point something along the lines of "spot check dockerization and docs one last time before merge" would be appropriate, and perhaps a second checkbox on the main "review" bullet point to ensure we get at least two maintainers looking at each new image. Thoughts?

[yosifkit]

A second reviewer spot would probably be appropriate. Not sure about the "final spot check" checkbox; perhaps the final "LGTM + build test" will serve this purpose? Should this file be pointed out in README.md?

[tianon]

Updated example:

Checklist for Review

• associated with or contacted upstream?
  • Adding F# language #1201 (comment)
• does it fit into one of the common categories? ("service", "language stack", "base distribution")
• is it reasonably popular, or does it solve a particular use case well?
• does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • Add F# to docs docs#583
• dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
• 2+ dockerization review?
• existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
  • Adding F# language #1201 (comment)
• if FROM scratch, tarballs only exist in a single commit within the associated history?

[tianon]

(and added a short note to README.md)

[yosifkit]

LGTM

[md5]

This looks like a good addition. I'm sure any necessary clarifications or improvements can be made incrementally.

One thing that would be good to address would be ensuring that the common tests pass and possibly inquiring about any additional tests that would be appropriate to add.

[tianon]

Good call, @md5; that kind of comes from the build test we do, but making it explicit is a good plan. 👍

[tianon]

LET'S DO THIS