[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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

Perl harmonized errors #149

Merged
merged 6 commits into from
Aug 16, 2023
Merged

Perl harmonized errors #149

merged 6 commits into from
Aug 16, 2023

Conversation

ehuelsmann
Copy link
Contributor
@ehuelsmann ehuelsmann commented Aug 15, 2023

🤔 What's changed?

Test suite for parse errors is now run too.

⚡️ What's your motivation?

Consistency. The rest is explained in #31.

🏷️ What kind of change is this?

🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

I think this is a minor version at most: the only difference in behaviour is the error code path, which now results in Cucumber Messages (in most cases) instead of exceptions to be caught.

The fact that the exceptions don't auto-stringify anymore doesn't look like a major issue, because they used to be immediately stringified when being thrown (throwing and having a stringification method turn out to be incompatible).

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@ehuelsmann ehuelsmann linked an issue Aug 15, 2023 that may be closed by this pull request
@ehuelsmann ehuelsmann requested a review from mpkorstanje August 15, 2023 00:52
@ehuelsmann
Copy link
Contributor Author
ehuelsmann commented Aug 15, 2023

Changelog still to come, obviously.

To do so, we need a generic parser error; overload ParserSingle for the purpose.

We used to throw this error at pickle time, which is also weird, but parse time is even more wrong,
because we need to cache the error being thrown in order to be able to throw the same error on each
line check (which means we got our layering wrong... ah, well).
@ehuelsmann ehuelsmann force-pushed the perl-harmonized-errors branch from 7a61565 to ae3391e Compare August 15, 2023 16:14
@ehuelsmann
Copy link
Contributor Author

After consulting with @mpkorstanje, he'll review in hindsight, if he wants to.

@ehuelsmann ehuelsmann merged commit b5fdf13 into main Aug 16, 2023
@ehuelsmann ehuelsmann deleted the perl-harmonized-errors branch August 16, 2023 09:39
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.

perl: Acceptance tests do not include parse errors
1 participant