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

Attempt to fix EventReader limitations. #203

Merged
merged 1 commit into from
Aug 31, 2016
Merged

Attempt to fix EventReader limitations. #203

merged 1 commit into from
Aug 31, 2016

Conversation

aaubry
Copy link
Owner

@aaubry aaubry commented Aug 25, 2016

No description provided.

EventReader was a wrapper over IParser that offered some abstractions for parsing,
but also had some design flaws. It has been replaced by extension methods for IParser.
The extension methods provide the same functionality,
and allow to always use the same type to represent the parser.
@joaope
Copy link

joaope commented Aug 30, 2016

Brilliant! Less wrapping is always good. And the breaking change is less visible as you use IParser all the way through.

@joaope
Copy link

joaope commented Aug 30, 2016

How/where do you usually document this breaking changes? If you do it at all.

Let me know if I can help to get this merged asap. Tks.

@aaubry
Copy link
Owner Author

aaubry commented Aug 31, 2016

The breaking changes are documented in the release notes, and also expressed in the version number. In this case, the major version number will be incremented, signaling a breaking change.
I plan to merge this later today unless I get feedback from other people, and I plan to release the next version, which will include this change, during this week. I have some other changes that I would like to include, and I need some more work to merge them.

@aaubry aaubry merged commit 796f91f into master Aug 31, 2016
@aaubry aaubry deleted the issue-149 branch May 15, 2017 09:23
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.

2 participants