Skip to content

Conversation

@segayuu
Copy link
Contributor

@segayuu segayuu commented Oct 19, 2018

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.

@segayuu segayuu requested a review from a team October 19, 2018 03:56
@segayuu segayuu changed the title Refactering Database#load() Change from map function to use event API Oct 20, 2018
@NoahDragon
Copy link
Member

Haven't tested yet, but the code change LGTM.

@tcrowe
Copy link
Contributor

tcrowe commented Jun 24, 2019

@segayuu Do you want to close this since it will not work after class refactor?

@segayuu
Copy link
Contributor Author

segayuu commented Jun 26, 2019

Well, I'll close it once and create it again after marging the class refactor.

@segayuu segayuu closed this Jun 26, 2019
@segayuu segayuu reopened this Jul 25, 2019
@segayuu segayuu force-pushed the refactoring-JSONparse branch 2 times, most recently from 6180ccd to b08c852 Compare August 29, 2019 14:12
@segayuu segayuu force-pushed the refactoring-JSONparse branch from b08c852 to 31726c8 Compare August 30, 2019 02:18
Copy link
Member

@SukkaW SukkaW left a 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.

Copy link
Member

@SukkaW SukkaW left a 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.

@SukkaW SukkaW merged commit 62bd5e8 into hexojs:master Jan 4, 2020
@SukkaW SukkaW mentioned this pull request Jan 4, 2020
@segayuu segayuu deleted the refactoring-JSONparse branch January 13, 2020 08:07
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.

4 participants