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.

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

[DO NOT MERGE] Configure esy monorepo workflow #42

Closed
wants to merge 5 commits into from

Conversation

andreypopp
Copy link
Contributor

@andreypopp andreypopp commented Dec 11, 2018

DO NOT MERGE

This PR explores esy monorepo workflow.

The main motivation is to provide a (hopefully) efficient workflow to develop a
group of packages together while keeping package structure (package boundaries)
maximally close to the one a consumer will be have when those packages are
installed from their corresponding releases.

Note that @esy-nightly/esy@0.4.4-31cf81 is required to try this:

% npm install -g @esy-nightly/esy@0.4.4-31cf81

The gist:

  • 764432f Configures esy monorepo, see commit's description for details.

  • 9c03952 Adds esymono command which wraps esy, see commit's description
    for details.

  • 5989ba8 Updates esy.lock (please ignore)

  • d095eb9 Fixes code to adhere to standard dune's set of compiler warnings (please ignore, this is just to make things work)

The workflow looks like (pasted from 9c03952 description):

Build all monorepo:

% ./esymono build

We can also build all monorepo in "release" mode (no devDependencies
provided and therefore we simulate the env in which packages will be built
when released):

% ./esymono build --release

We can build only a specific package by referring to it by its name:

% ./esymono build @reason-native/rely

Or we can refer by the manifest:

% ./esymono build ./rely.json

Which maybe more convenient as shell autocompletes paths by default.

To execute an arbitrary COMMAND in a dev environment w/o waiting for
packages in the monorepo to be built:

% ./esymono vim .

To execute an arbitrary COMMAND in a dev environment but before ensuring
that packages are built:

% ./esymono x TestPastel.exe

So this can be used to run tests.

It's also possible to narrow the environment to the specific package:

% ./esymono x-in ./rely.json echo '#{self.name}'

Note that the idea for esymono is to be subsumed in some form into esy.
Possibly via a special configuration put into .esyrc. This is just a preview.

The idea is that `esy.json` links to all packages in the monorepo.

For each `<pkg>` it has:

    "dependencies": {
      "<pkg>": "*",
      ...
    },
    "resolutions": {
      "<pkg>": "link:./<pkg>.json",
      ...
    }

Also we add a little helper `esydune` which configures `dune` given the
esy environment for the package. Thus each package has

    "esy": {
      "build": "esydune build",
      "build": "esydune install"
    }

as its `"esy"` configuration.

We also remove `"devDependencies"` configuration from all packages but
the root one (`esy.json`). As we are going to develop packages as a
whole we want to configure `"devDependencies"` only ath the root package
of the monorepo and then make them available to each package's build
environment.

Thanks for composable dune CLI we even can invoke aarbitrary `dune` commands
that way and they will be scoped for the current package rather than for
the whole dune project (which don't want to use because we are using
esy-way of stitching packages together):

    % esydune runtest
    esydune: dune runtest --root . --only-packages "$cur__name"
This commit adds `esymono` command which wraps `esy` to configure monorepo
workflow.

The main reason we need this is because we want `"devDependencies"` configured
at the root package's `esy.json` be available to each package's build
environment. Therefore we represent common esy commands (`esy build`, ...) via
esy low level commands invocation which sets a dependency specification
(DEPSPEC).

For example this command will build all dependencies in the monorepo
providing root's devDeps to each package build environment:

    % esy build-dependencies \
        --link-depspec 'dependencies(self)+devDependencies(root)' \
        --all \
        root

Or via `esymono`:

    % esymono build

We can also build all monorepo in "release" mode (no devDependencies provided
and therefore we simulate the env in which packages will be built when
released):

    % esymono build --release

We can build only a specific package by referring to it by its name:

    % esymono build @reason-native/rely

Or we can refer by the manifest:

    % esymono build ./rely.json

Which maybe more convenient as shell autocompletes paths by default.

To execute an arbitrary `COMMAND` in a dev environment w/o waiting for packages
in the monorepo to be built:

    % esymono vim .

To execute an arbitrary `COMMAND` in a dev environment but before ensuring that
packages are built:

    % esymono x TestPastel.exe

So this can be used to run tests.

It's also possible to narrow the environment to the specific package:

    % esymono x-in ./rely.json echo '#{self.name}'
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 11, 2018
@@ -8,7 +8,7 @@
open TestFramework;
open Pastel;

describe("Pre test runner exhaustiveness test", ({test}) =>
describe("Pre test runner exhaustiveness test", ({test, _}) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this change. I can understand the point of warning 9, and why it might be helpful in some cases, but this is explicitly not one of those cases. We don't really care about making sure each field in the record is listed and designed the API in that way on purpose.

Are there examples of this being an important thing that catches bugs? I can understand theoretically how it might come up, but it never seems relevant in practice.

I wonder what some alternatives might be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I didn't mean to suggest merging this at all, just to make things roll. Please see first two commits and ignore the last two.

As for warning 9 in this repo I think we can just configure dune to the needed set of warnings.

As for warning 9 in general — I commented here
#38 (comment) — don't have examples of bugs it prevented right now though. I'll dig something.

Copy link
Contributor

@kyldvs kyldvs Dec 11, 2018

Choose a reason for hiding this comment

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

Yeah sorry - totally understand that. Glad to see this rolling, and this was meant to be a comment more generally about warning 9, not this particular PR. The conversation is probably best for the issue you linked instead of here!

@bandersongit
Copy link
Contributor

I like the general idea a lot, Jordan and I tried something like this initially but ran into performance issues, I'm glad that you are tackling this problem.

@andreypopp
Copy link
Contributor Author

Replaced with #74

@kyldvs kyldvs deleted the andreypopp/esymono branch February 5, 2019 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants