Skip to content

Added default arguments transpilation #9

Merged
stropitek merged 6 commits into
mljs:masterfrom
glebmachine:master
Nov 9, 2018
Merged

Added default arguments transpilation #9
stropitek merged 6 commits into
mljs:masterfrom
glebmachine:master

Conversation

@glebmachine
Copy link
Copy Markdown
Contributor

Fix of #8

@glebmachine
Copy link
Copy Markdown
Contributor Author

glebmachine commented Nov 8, 2018

Waiting forward your approvals, guys, thank you!

@stropitek
Copy link
Copy Markdown
Member

I'm ok with that

@targos should we keep the CJS build as well?

@stropitek stropitek requested a review from targos November 9, 2018 08:06
@targos
Copy link
Copy Markdown
Member

targos commented Nov 9, 2018

I think we need to keep a cjs build for Node?

@glebmachine
Copy link
Copy Markdown
Contributor Author

Will fix that, back soon!

Copy link
Copy Markdown
Member

@stropitek stropitek left a comment

Choose a reason for hiding this comment

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

@glebmachine can you create 2 builds? One to commonjs in lib and one to es6 modules in a new directory called lib-es6?

@glebmachine
Copy link
Copy Markdown
Contributor Author

@stropitek Done!

@glebmachine
Copy link
Copy Markdown
Contributor Author

I will be very appreciated if you accept the request today. I have release date today(
Thank you and excuse me for being annoying
@stropitek

@stropitek
Copy link
Copy Markdown
Member

stropitek commented Nov 9, 2018

two more things:

  • Can you remove and ignore the yarn.lock file and update the package-lock file?
  • Can you change change lib-esm to lib-es6? It's just to be consistent across our packages!

Thank you!

@targos
Copy link
Copy Markdown
Member

targos commented Nov 9, 2018

I'm working on it

@targos
Copy link
Copy Markdown
Member

targos commented Nov 9, 2018

I pushed a change to

  • rename the folder to lib-es6
  • remove yarn.lock
  • use the preset-env instead of a specific plugin to provide more compatibility

Copy link
Copy Markdown
Member

@stropitek stropitek left a comment

Choose a reason for hiding this comment

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

LGTM

@glebmachine
Copy link
Copy Markdown
Contributor Author

Thank you guys!

@targos
Copy link
Copy Markdown
Member

targos commented Nov 9, 2018

@stropitek can you merge (squash) and release please?

@stropitek
Copy link
Copy Markdown
Member

yep!

@stropitek stropitek merged commit 50f16bf into mljs:master Nov 9, 2018
@stropitek
Copy link
Copy Markdown
Member

@glebmachine

I have published the packages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants