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

Remove the deprecated log_syntax_violation function for 2.0 #511

Closed
wants to merge 1 commit into from

Conversation

@est31
Copy link
Contributor

est31 commented Jul 13, 2019

I've also removed the OldFn variant as that function was the only place that variant was constructed.
Does that make sense or should I replace the entire enum with Option's everywhere like it was before #433?


This change is Reviewable

@dekellum
Copy link
Contributor

dekellum commented Jul 14, 2019

If you replaced the enum with an Option, you'd still need to wrap in a new-type (generic over the lifetime) to implement the same specific methods for the type, right? Looks good to me, the way you have it.

@SimonSapin
Copy link
Member

SimonSapin commented Jul 15, 2019

Merged in the 2.0 branch, with an Option: 48f3bed.

Thanks!

@SimonSapin SimonSapin closed this Jul 15, 2019
@dekellum
Copy link
Contributor

dekellum commented Jul 15, 2019

…or move those methods to Parser and don't bother using them from Input. That works too, I see, and is also more compact.

@est31
Copy link
Contributor Author

est31 commented Jul 15, 2019

Merge commit was: 90e65b2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.