| Re: [mat-dev] Parallel parsing |
|
Hi, >>
This is an exciting contribution, please take a look. First of all - @Jason thanks a lot for the contribution! It is definitely an exciting one! >> I estimate there are about 3000 lines of changes (though some are lines just moved to new files), so probably needs to go through the
large contribution process. I tried to clarify the process part and I don’t think we need a CQ. Formally, the process is required for contributions (single changes) of above 1000 lines. Also, its purpose is to help in case of doubts that code may
be copied inappropriately from somewhere. Jason, you’ve done a really great job in splitting the changes in small, easy to review commits! I went quickly through the changes and saw the three bigger ones are actually moving MAT’s existing coding around. The
smaller ones were also very MAT specific, I have no concerns about unproperly copied code. Therefore, I think from the process perspective we are good to go (without a CQ), and we may start reviewing/merging the pieces. About the content - from what I’ve seen so far, the most critical part is indeed the moved Index builders, as this is an API breaking change. We’ll have to figure out a reasonable way to deal with it. I guess sooner or later we’ll have to go to a 2.0 version of MAT and clean up the APIs, but I wouldn’t mix such activity with the current contribution. I guess we’ll need some more time for the reviews, so please bear with us. I nevertheless wanted to thank Jason for the change and give an update on the process part. Regards Krum |