Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Catch RubyParser exceptions #62

Merged
merged 1 commit into from
Dec 29, 2015
Merged

Catch RubyParser exceptions #62

merged 1 commit into from
Dec 29, 2015

Conversation

wfleming
Copy link
Contributor

Depending on what causes RubyParser to fail, the exception could be from
Racc or from RubyParser, so we should catch these as well.

@codeclimate/review @ABaldwinHunter

Depending on what causes RubyParser to fail, the exception could be from
Racc or from RubyParser, so we should catch these as well.
@gdiggs
Copy link
Contributor

gdiggs commented Dec 29, 2015

LGTM. Funny that CC failed with this

@ABaldwinHunter
Copy link
Contributor

Nice

@wfleming
Copy link
Contributor Author

Um...yeah, I'm trying to think of how to explain the CC failure before I merge because it concerns me.

CC was green on #60, which was the previous version of the duplication engine (b226). The later builds on #61 and #62 have been the newest duplication engine release (b258), and they've failed on parser errors. (On the same commit, even: compare this build and this build).

Gem versions have not been changed in months, so they should be identical, and parser results shouldn't vary. And the changes in #60 aren't even involved with Ruby parsing at all (that happens in-line, unlike other languages that shell out to their respective parsers).

So that's weird & I don't know how to explain it yet.

@wfleming
Copy link
Contributor Author

Ugh. I've explained it: the prior duplication engine (b226) was still the very old one that just swallowed every exception ever. So of course it didn't fail.

wfleming added a commit that referenced this pull request Dec 29, 2015
@wfleming wfleming merged commit 371a7cf into master Dec 29, 2015
@wfleming wfleming deleted the will/catch-parse-error branch December 29, 2015 16:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants