Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

Add lerna and split project #138

Merged
merged 22 commits into from
Nov 28, 2017
Merged

Add lerna and split project #138

merged 22 commits into from
Nov 28, 2017

Conversation

nlepage
Copy link
Member

@nlepage nlepage commented Nov 16, 2017

Description

  • Add lerna and move sources in a package
  • Move scripts in top level package.json
  • Split the project
  • Fix all... (documentation, tests, lint, coverage, ci)

Issue : #78

@nlepage nlepage added this to the 1.0.0-RC1 milestone Nov 16, 2017
@nlepage nlepage self-assigned this Nov 16, 2017
@nlepage
Copy link
Member Author

nlepage commented Nov 16, 2017

Codecov seems pretty lost...
Last execution reported an error : https://codecov.io/github/Zenika/immutadot/commit/fb0dfc641e0302057f34707bc358bc44f236b765
Either the report is empty, or it fails to find source files...

@nlepage
Copy link
Member Author

nlepage commented Nov 16, 2017

yarn test crashes in immutadot-lodash
Module resolver can't find imports relative to immutadot/src...

@codecov-io
Copy link

codecov-io commented Nov 17, 2017

Codecov Report

Merging #138 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #138   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          75     75           
  Lines         322    278   -44     
  Branches       54      0   -54     
=====================================
- Hits          322    278   -44
Impacted Files Coverage Δ
packages/immutadot/src/flow/flow.js 100% <ø> (ø)
packages/immutadot/src/core/path.utils.js 100% <ø> (ø)
packages/immutadot-lodash/src/array/remove.js 100% <ø> (ø)
packages/immutadot/src/math/multiply.js 100% <ø> (ø)
packages/immutadot-lodash/src/object/omit.js 100% <ø> (ø)
packages/immutadot/src/array/concat.js 100% <ø> (ø)
packages/immutadot-lodash/src/object/pickBy.js 100% <ø> (ø)
packages/immutadot/src/lang/toggle.js 100% <ø> (ø)
...kages/immutadot-lodash/src/array/takeRightWhile.js 100% <ø> (ø)
packages/immutadot-lodash/src/collection/map.js 100% <ø> (ø)
... and 66 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad0071b...b318f6e. Read the comment docs.

@nlepage
Copy link
Member Author

nlepage commented Nov 17, 2017

OK for some reason I had removed module-resolver from babel's build (when making flow), and this was a huge mistake.
I put it back, and it fixed immutadot-lodash's tests.

It made me realise that immutadot-lodash's tests rely on the built immutadot core.
So it's necessary to execute yarn build on immutadot core before executing immutadot-lodash's tests for these to work...

For now I see no clean way of handling this.
This is a dependency between scripts of different packages, I don't think lerna can help us resolve this...
Moreover it would be pretty ugly to add a call to yarn build in a pretest script...

@nlepage
Copy link
Member Author

nlepage commented Nov 17, 2017

Codecov is back on his feet, but it reports -45 files so it is missing one of the reports...

@nlepage
Copy link
Member Author

nlepage commented Nov 27, 2017

Maybe tests should be rewritten to real unit tests, so that these don't rely on immutadot's core...

@nlepage nlepage mentioned this pull request Nov 27, 2017
4 tasks
@nlepage
Copy link
Member Author

nlepage commented Nov 28, 2017

Finally codecov is OK. But codecov ignores wouldn't work, I had to use jest ignores. And codecov flags still aren't working...

@frinyvonnick
Copy link
Contributor

When i execute yarnto install dependencies i have the following warning:

warning "workspace-aggregator-d2639e58-f6c2-4c62-a6fd-4cb79bd5a18f > immutadot@0.3.2" has unmet peer dependency "lodash@^4.17.4".

Then when i execute yarn test i have a bunch of errors like:

FAIL packages/immutadot-lodash/src/string/capitalize.spec.js
  ● Test suite failed to run

    Cannot find module 'immutadot/core/convert' from 'capitalize.js'

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:191:17)
      at Object.<anonymous>.Object.defineProperty.value (packages/immutadot-lodash/src/string/capitalize.js:1:335)
      at Object.<anonymous> (packages/immutadot-lodash/src/string/capitalize.js:1:495)

@nlepage
Copy link
Member Author

nlepage commented Nov 28, 2017

You have to execute a yarn build for yarn test to work.
This is due to immutadot-lodash's tests needing the built immutadot core.

@frinyvonnick
Copy link
Contributor

frinyvonnick commented Nov 28, 2017

Maybe we could add a pretest script for this to be transparent for developper ?

@nlepage
Copy link
Member Author

nlepage commented Nov 28, 2017

As I mentionned in #138 (comment) I find it a little overkill, and I'm not sure if it's possible to have inter-package script dependency...

@nlepage nlepage changed the title 🚧 Add lerna and split project Add lerna and split project Nov 28, 2017
@nlepage
Copy link
Member Author

nlepage commented Nov 28, 2017

@frinyvonnick I added immutadot's build to immutadot-lodash's pretest, this is ready for review.

@frinyvonnick
Copy link
Contributor

Nice work 👍

@nlepage nlepage merged commit 345146d into master Nov 28, 2017
@nlepage nlepage deleted the feature/78 branch November 28, 2017 21:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants