Re: [mat-dev] Parallel parsing

[Jason Koch <jkoch@xxxxxxxxxxx>]

Hi Krum & Andrew

I noticed most of the patch set has some comments now. I think I have responded to the majority of them now, and my belief is the overall direction is positive with the bulk of the concern around the API compatibility changes.

Can you let me know how you'd like to proceed?

I can either bundle up a new set of changes over the top, or I can try to rework the commits in basically the order they were presented. (Or another order?).

Also, I did understand from the docs and prior emails with Andrew that I should have API compat issues enabled and flagged, however when I used Eclipse refactor to extract classes it seems to have not treated this as an issue.

Is there a concrete / straightforward way that I can verify the API compatibility from an eclipse perspective between two branches?

Thanks

Jason

On Thu, Mar 14, 2019 at 3:18 AM Tsvetkov, Krum <krum.tsvetkov@xxxxxxx> wrote:

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

 

_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/mat-dev