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

Improve error message when an object is passed where a record is expected #7101

Merged
merged 2 commits into from
Oct 13, 2024

Conversation

zth
Copy link
Collaborator

@zth zth commented Oct 13, 2024

This improves the error message when an object is passed where a record is expected. A scenario that happens from time to time when copy pasting JS/TS stuff.

@zth zth requested review from cknitt and cristianoc October 13, 2024 16:50
@zth zth force-pushed the error-msg-pass-object-to-record branch from 0bb7d09 to c45700e Compare October 13, 2024 16:51
Copy link
Member

@cknitt cknitt left a comment

Choose a reason for hiding this comment

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

👍

@zth zth enabled auto-merge (squash) October 13, 2024 16:52
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Syntax Benchmarks

Benchmark suite Current: c45700e Previous: e1b7fb7 Ratio
Parse RedBlackTree.res - time/run 1.2079324133333333 ms 1.2123143266666667 ms 1.00
Parse RedBlackTree.res - allocs/run 199057 words 199057 words 1
Print RedBlackTree.res - time/run 2.31255812 ms 2.3104395133333333 ms 1.00
Print RedBlackTree.res - allocs/run 236403 words 236403 words 1
Print RedBlackTreeNoComments.res - time/run 2.085011106666667 ms 2.10057036 ms 0.99
Print RedBlackTreeNoComments.res - allocs/run 249186 words 249186 words 1
Parse Napkinscript.res - time/run 39.036746519999994 ms 39.28006235333333 ms 0.99
Parse Napkinscript.res - allocs/run 8670066 words 8670066 words 1
Print Napkinscript.res - time/run 75.60905208 ms 77.00100409999999 ms 0.98
Print Napkinscript.res - allocs/run 9776327 words 9776327 words 1
Parse HeroGraphic.res - time/run 5.09393054 ms 5.13472718 ms 0.99
Parse HeroGraphic.res - allocs/run 1219326 words 1219326 words 1
Print HeroGraphic.res - time/run 8.713838513333334 ms 8.775952553333333 ms 0.99
Print HeroGraphic.res - allocs/run 1396466 words 1396466 words 1

This comment was automatically generated by workflow using github-action-benchmark.

@zth zth merged commit e050527 into master Oct 13, 2024
19 checks passed
@cristianoc
Copy link
Collaborator

Syntax Benchmarks

Benchmark suite Current: c45700e Previous: e1b7fb7 Ratio
Parse RedBlackTree.res - time/run 1.2079324133333333 ms 1.2123143266666667 ms 1.00
Parse RedBlackTree.res - allocs/run 199057 words 199057 words 1
Print RedBlackTree.res - time/run 2.31255812 ms 2.3104395133333333 ms 1.00
Print RedBlackTree.res - allocs/run 236403 words 236403 words 1
Print RedBlackTreeNoComments.res - time/run 2.085011106666667 ms 2.10057036 ms 0.99
Print RedBlackTreeNoComments.res - allocs/run 249186 words 249186 words 1
Parse Napkinscript.res - time/run 39.036746519999994 ms 39.28006235333333 ms 0.99
Parse Napkinscript.res - allocs/run 8670066 words 8670066 words 1
Print Napkinscript.res - time/run 75.60905208 ms 77.00100409999999 ms 0.98
Print Napkinscript.res - allocs/run 9776327 words 9776327 words 1
Parse HeroGraphic.res - time/run 5.09393054 ms 5.13472718 ms 0.99
Parse HeroGraphic.res - allocs/run 1219326 words 1219326 words 1
Print HeroGraphic.res - time/run 8.713838513333334 ms 8.775952553333333 ms 0.99
Print HeroGraphic.res - allocs/run 1396466 words 1396466 words 1
This comment was automatically generated by workflow using github-action-benchmark.

@cknitt consistency looks surprisingly good.
I guess we can leave this on for a while, and if it stays like this put a rather small threshold and turn off commenting by default.

@cknitt cknitt deleted the error-msg-pass-object-to-record branch October 13, 2024 17:20
@cknitt
Copy link
Member

cknitt commented Oct 13, 2024

I guess we can leave this on for a while, and if it stays like this put a rather small threshold and turn off commenting by default.

It might be nice to have the benchmark outputs always visible. So that

  • one sees the benchmarking action is still working and not failing silently
  • people get motivated to add more benchmarks 🙂

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