Skip to content

Manifest: protocol packages

Romain requested to merge nomadic-labs/tezos:romain-manifest-protos into master

Context

After this MR, all dune and .opam files are generated from the manifest, except:

  • those in lib_protocol directories (this is a TODO);
  • and a few others that we don't intend to generate (see let exclude at the end of manifest/main.ml).

How to Review

Here are the main points to review.

  • The diff in dune and .opam files: the goal is to be mostly equivalent to master. The diff is actually small in this MR because we already merged most of it, so this should be easy. This is concentrated in the commit: Manifest: generate
  • The fact that manifest/main.ml is consistent with generated files. This is trivial: it is already checked by the CI. But you can run make -C manifest and see that git diff shows nothing.
  • How maintainable are the definitions of protocol targets in manifest/main.ml. This is concentrated in the commit: Manifest: protocols and is the main part of this MR. Since you already know that the code is correct (see above 2 points), you can focus on the style, the organization of the code, and in how we use ifs to encode variations depending on protocol numbers.
  • Why we remove some .opam files in commit Build: remove unused .opam files. If the code compiles, it means that we don't have dune files that reference those .opam files in (package) stanzas. If the opam tests in the CI pass, it means that no opam package depends on those .opam. That should be enough to convince you.
  • Finally, commit Manifest: do not exclude protocol files makes sure that we don't introduce new dune or .opam files without the manifest to generate them. You can try to add such a file, run make -C manifest, and see that it fails. It was already the case for all folders except proto_* folders and a couple other exceptions. After this MR, it fails for all folders except lib_protocol folders and the same couple of other exceptions.

Manually testing the MR

The CI should pass. It runs make -C manifest to check that there is no diff.

Checklist

  • Document the interface of any function added or modified (see the coding guidelines)
  • Provide automatic testing (see the testing guide).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR
Edited by Romain

Merge request reports