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.

Sign up for GitHub

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

Jump to bottom

fix: respect quiet flag on ts build #605

Merged
merged 1 commit into from
Oct 23, 2020
Merged

fix: respect quiet flag on ts build #605

merged 1 commit into from
Oct 23, 2020

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Oct 17, 2020

Fix #603

styfle
styfle reviewed Oct 19, 2020
View reviewed changes
src/typescript.js Outdated Show resolved Hide resolved
styfle
styfle reviewed Oct 19, 2020
View reviewed changes
src/typescript.js Outdated Show resolved Hide resolved
@huozhi huozhi requested a review from styfle October 20, 2020 08:27
styfle
styfle reviewed Oct 20, 2020
View reviewed changes
src/index.js Outdated Show resolved Hide resolved
styfle
styfle reviewed Oct 20, 2020
View reviewed changes
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

We also need a test so we dont regress

@huozhi huozhi requested a review from styfle October 20, 2020 15:45
styfle
styfle previously approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@styfle styfle added the automerge Automatically merge PR once checks pass label Oct 20, 2020
@huozhi
Copy link
Member Author

huozhi commented Oct 20, 2020

one extra question not related to this PR but I encounter when I test locally: how could I build the latest ncc changes to dist? it seems yarn build doesn't generate the latest code from src to dist. did I miss any steps?

@huozhi
Copy link
Member Author

huozhi commented Oct 22, 2020 •
edited

@styfle this solution seems not working. the cliargs that typescript.js file get is always empty, but the cliargs.consume did get called before it. but I don't know why..

will adding a new env var like process.env.NCC_QUIET be acceptable? you might want to remove the automerge label

@styfle styfle removed the automerge Automatically merge PR once checks pass label Oct 23, 2020
@styfle styfle dismissed their stale review October 23, 2020 15:09

Outdatedd

@styfle
Copy link
Member

styfle commented Oct 23, 2020

@huozhi Let's use process.env.__NCC_OPTS = JSON.stringify(opts) and then the appropriate files can deserialize and use any necessary options

@huozhi huozhi requested a review from styfle October 23, 2020 15:49
styfle
styfle approved these changes Oct 23, 2020
View reviewed changes
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@styfle styfle added the automerge Automatically merge PR once checks pass label Oct 23, 2020
@kodiakhq kodiakhq bot merged commit 4456076 into vercel:master Oct 23, 2020
@huozhi huozhi deleted the fix-quite branch October 23, 2020 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@styfle styfle styfle approved these changes

@Timer Timer Awaiting requested review from Timer

Assignees
No one assigned
Labels
automerge Automatically merge PR once checks pass hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Typescript build doesn't respect the quiet flag
2 participants
@huozhi @styfle