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

Flow #134

Merged
merged 5 commits into from
Nov 15, 2017
Merged

Flow #134

merged 5 commits into from
Nov 15, 2017

Conversation

nlepage
Copy link
Member

@nlepage nlepage commented Nov 13, 2017

Description

First version of flow.

Issue : #133

@nlepage nlepage added this to the 1.0.0-RC1 milestone Nov 13, 2017
@nlepage nlepage self-assigned this Nov 13, 2017
@codecov-io
Copy link

codecov-io commented Nov 13, 2017

Codecov Report

Merging #134 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #134   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          74     75    +1     
  Lines         259    322   +63     
  Branches       54     54           
=====================================
+ Hits          259    322   +63
Impacted Files Coverage Δ
src/math/divide.js 100% <ø> (ø) ⬆️
src/object/mapKeys.js 100% <ø> (ø) ⬆️
src/object/omit.js 100% <ø> (ø) ⬆️
src/array/difference.js 100% <ø> (ø) ⬆️
src/array/concat.js 100% <ø> (ø) ⬆️
src/collection/sortBy.js 100% <ø> (ø) ⬆️
src/object/mapValues.js 100% <ø> (ø) ⬆️
src/collection/filter.js 100% <ø> (ø) ⬆️
src/array/takeWhile.js 100% <ø> (ø) ⬆️
src/array/xorBy.js 100% <ø> (ø) ⬆️
... and 113 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 6dfeca4...03583dc. Read the comment docs.

@nlepage
Copy link
Member Author

nlepage commented Nov 13, 2017

@frinyvonnick What do you think ? Not sure about the code arrangement. Only the core namespace available for now.

@nlepage nlepage changed the title Flow 🚧 Flow Nov 13, 2017
@nlepage
Copy link
Member Author

nlepage commented Nov 15, 2017

Hey @frinyvonnick @charlyx ! Like we talked about earlier today I managed to generate all of the flow exports of the curried functions.

@nlepage
Copy link
Member Author

nlepage commented Nov 15, 2017

@frinyvonnick What do you think ? Should we merge as is and open a new issue to improve flow and avoid multiple reinstanciations ?

Copy link
Contributor

@frinyvonnick frinyvonnick left a comment

Choose a reason for hiding this comment

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

I love what you did there. I think we can optimize in another issue 👍


nsItems.forEach(async ({ name }) => await writeFile(
path.resolve(nsDir, `${name}.js`),
`import { ${name} } from '${namespace}/${name}'
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is a bit weird here 😕 and in further generated code parts.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know but it allows to have a correct indentation in the generated files.
I haven't found a simple way of having a nicer indentation of the templates...
I think we can let it as it is and try to improve it later ?


await writeFile(
path.resolve(nsDir, 'index.js'),
`${nsItems.map(({ name }) => `import { ${name} } from './${name}'`).join('\n')}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can directly export all elements of the module here. Like :

 `export * from './${name}'`

@nlepage nlepage changed the title 🚧 Flow Flow Nov 15, 2017
@nlepage nlepage merged commit 6c64014 into master Nov 15, 2017
@nlepage nlepage deleted the feature/133 branch November 15, 2017 21:55
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