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

build!: don't ship untranspiled code or commit transpiled code #4

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

RyanZim
Copy link
Collaborator

@RyanZim RyanZim commented Nov 8, 2022

After #3

@RyanZim RyanZim requested a review from mvayngrib November 8, 2022 01:18
@mvayngrib
Copy link

mvayngrib commented Nov 8, 2022

tests are failing cause make-dir-cli is an ESM module which node 6 doesn't like nm, i re-read your msg above

Copy link

@mvayngrib mvayngrib left a comment

Choose a reason for hiding this comment

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

utACK

@RyanZim RyanZim force-pushed the ryan/build-package branch from 125b8c5 to e6128b2 Compare November 8, 2022 16:29
@RyanZim
Copy link
Collaborator Author

RyanZim commented Nov 8, 2022

Rebased on #3; tests should pass now.

"repository": {
"type": "git",
"url": "git+https://github.com/ExodusMovement/buffer-noise.git"
},
"scripts": {
"build": "flow-remove-types ./src/index.js > ./lib/index.js",
"build": "make-dir lib && flow-remove-types ./src/index.js > ./lib/index.js",
Copy link

@fboucquez fboucquez Nov 8, 2022

Choose a reason for hiding this comment

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

If we are not shipping transpiling code, why are we building? Should lint be enough to detect any issue babel can detect? Can we remove babel? (if jest manage to run untranpiled code)

There are general questions, no just for this PR

ufff, missread the build. ignore this please

@RyanZim RyanZim merged commit acc7def into master Nov 8, 2022
@RyanZim RyanZim deleted the ryan/build-package branch November 8, 2022 16:35
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