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

Configure ocular to build modules in deeper level subfolders. #32

Closed
kenns29 opened this issue Sep 14, 2019 · 3 comments
Closed

Configure ocular to build modules in deeper level subfolders. #32

kenns29 opened this issue Sep 14, 2019 · 3 comments
Assignees
Labels
dev exp Development experience

Comments

@kenns29
Copy link
Collaborator

kenns29 commented Sep 14, 2019

ocular-build seems to only work for modules in the immediate subfolder, e.g. modules/ in the current monorepo. However, it doesn't seem to build modules within deeper level subfolders, e.g. bindings/jupyter-modules, as shown in this PR (#25). Currently, modules within jupyter-modules seem to only use lerna run build instead. This behavior introduces some inconsistencies between modules inside the jupyter-modules/ folder and the modules inside the modules/ folder. Either ocular has to be changed to allow building modules within deeper level subfolders or jupyter-modules/ have to be moved to the root level to accommodate the current ocular-build behavior. @ibgreen may have an idea on which is a better way to do this.


Pull Request (uber-web/ocular#261)

@kenns29 kenns29 added the rfc Request for comments label Sep 14, 2019
@ibgreen
Copy link

ibgreen commented Sep 14, 2019

If lerna handles recursive modules trees, it may not be too hard to update ocular to handle it.

We'd also want to verify that our "yarn workspaces" setup handles it, but I would expect that it does.

Finally, the ocular shell scripts (like ocular-bootstrap, ocular-build) currently iterate over the modules directory and does some fixup and processing, so those shell scripts would need to be modified.

You would need to put together that PR on ocular, but I could advise.

That said, if just one module, I'd suggest just moving it up in the tree for now.

@kenns29
Copy link
Collaborator Author

kenns29 commented Sep 14, 2019

Current workspace setup has both modules/ and bindings/jupyer-modules/:

{
  "lerna": "3.14.1",
  "version": "1.0.0",
  "packages": ["modules/*", "bindings/jupyter-modules/*"],
  "npmClient": "yarn",
  "useWorkspaces": true
}

This enables lerna to build on both subfolders at once. I think it's a good idea to update ocular-build to match lerna's behavior.

@kenns29 kenns29 self-assigned this Sep 16, 2019
@kenns29 kenns29 added dev exp Development experience and removed rfc Request for comments labels Oct 5, 2019
kenns29 added a commit to uber-web/ocular that referenced this issue Oct 9, 2019
…urrently only applicable to ocular-build (#261)

Addresses the issue in uber/manifold#32

An optional field `modules` is now processed if added into `ocular-dev-tools.config.js`, similar to the `packages` field in `lerna.json`. Currently, this option is only applicable to `ocular-build`.

**Example Usage**:

in `ocular-dev-tools.config.js`
```javascript
module.exports = {
 ...
 modules: ['modules/*', 'bindings/pydeck-modules/*']
};
```

**New Behaviors**:
1) if the `modules` field is empty, no new behavior is added.
2) if the `modules` field is provided, it will override the default modules folder for `ocular-build`, which is `modules`.
3) if there are module list provided as the parameter for `ocular-build`, e.g. `ocular-build module1,module2`, then this list will override the `modules` setting during `ocular-build`.
4) `ocular-build` now has a new dependency on `jq`, which is not out-of-box in bash (the user has to install `jq`). But this dependency is already prevalent in other ocular commands.

**Breaking Change**:
1) The module list parameter for `ocular-build` now has to be the full relative path to the repo, e.g. `ocular-build core,json,react` will have to changed to `ocular-build modules/core,modules/json,modules/react`.

**FAQ**:
1) Q: Are there plans to make the `modules` settings work with other commands such as `ocular-publish`.
    A: Yes
2) Q: Why not simply use the `packages` setting in lerna.json.
     A: Ocular is supposed to be a standalone tool independent of `lerna`, although it may use some lerna features (it currently depends on lerna), but those features should be exposed as ocular features to the end-user. Ocular should not require a `lerna.json` file to work. Thus, ocular needs to have its own `modules` settings.
@Firenze11
Copy link
Contributor

Closing it since it is resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev exp Development experience
Projects
None yet
Development

No branches or pull requests

3 participants