-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Change from map function to use event API #32
Conversation
Haven't tested yet, but the code change LGTM. |
@segayuu Do you want to close this since it will not work after |
Well, I'll close it once and create it again after marging the class refactor. |
6180ccd
to
b08c852
Compare
…factoring-JSONparse
b08c852
to
31726c8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM. I might setup a benchmark to see if event API has any potential performance boost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has been tested with a dummy Hexo site, it works fine.
By switching to the
data
event handler (and header, footer), unnecessary branching is reduced.In addition, the second argument of
JSONStream.parse()
is a map for processing all data, it is not suitable as a place to consume.