Skip to content

Remove unnecessary Output.flushs before Global.exit and Global.crash - #535

Merged
Jarred-Sumner merged 4 commits into
mainfrom
unknown repository
Jul 10, 2022
Merged

Jarred-Sumner merged 4 commits into
mainfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jul 10, 2022 •

Copy link
Copy Markdown

I thought about this a bit more and realized that we could remove Global.exit's code parameter because in 99.5% of all cases we only ever exit with either 1 or 0, so we could actually make Global.crash() always exit with 1 (which it does already) and Global.exit() always exit with 0, indicating failure and success, respectively. We might also want to rename Global.crash to Global.fail because Global.crash can mislead people that it's only called when a bug is encountered. And we should explicitly mention in the docs of those two that they already flush output. This way we can prevent these mistakes in the future.
Or maybe we could only have one function that exits and we let it take an enum. So you can do Global.exit(.error) and Global.exit(.success). I think that might actually be the best and most readable solution.
In fact we could make it a union(enum) and allow exit(.{ .custom = 50 }); too for codes that are not 0 or 1.
Should I do the above in a followup PR?
This PR would be in preparation for that.

Additionally, the first commit fixes this issue I found:

$ bun run
No "scripts" in package.json found.$ 

(missing newline)

@ghost

ghost commented Jul 10, 2022

Copy link
Copy Markdown
Author

@Jarred-Sumner
Jarred-Sumner merged commit 16452c1 into oven-sh:main Jul 10, 2022
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Thank you

@ghost

ghost commented Jul 10, 2022 •

Copy link
Copy Markdown
Author

@Jarred-Sumner thank you for merging.

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.

1 participant