sccn / roiconnect

ROI connectivity analysis in EEG
39 stars 17 forks source link

Fix bugs from merge conflicts #58

Closed nguyen-td closed 1 year ago

nguyen-td commented 1 year ago
          Makes a lot of sense, thanks for the change. I just realized that you may have introduced duplicate lines in your last PR, perhaps while resolving conflicts (which I just saw):

https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L264-L274

It's basically the same as

https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L249-L258

I remember fixing line 270 so 264-274 can be deleted.

I also fixed the following line https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L278 to fc_name = g.methods{fc}; It would be great if you reverted that too.

Originally posted by @nguyen-td in https://github.com/sccn/roiconnect/issues/57#issuecomment-1645029969

arnodelorme commented 1 year ago

Yes, my apologies. I was struggling with merge conflict and I probably did not solve them correctly. If you can do what you can and I will double check the function.

On Jul 21, 2023, at 6:31 AM, Tien Dung Nguyen @.***> wrote:

      Makes a lot of sense, thanks for the change. I just realized that you may have introduced duplicate lines in your last PR, perhaps while resolving conflicts (which I just saw):

https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L264-L274

It's basically the same as

https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L249-L258

I remember fixing line 270 so 264-274 can be deleted.

I also fixed the following line https://github.com/sccn/roiconnect/blob/ab733e552a0b5b13170195c97d9d8a9283458968/pop_roi_connect.m#L278 to fc_name = g.methods{fc}; It would be great if you reverted that too.

Originally posted by @nguyen-td in #57 (comment)

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

nguyen-td commented 1 year ago

Yes, no problem, I assigned myself anyway, it was just a note (to myself). I will take care of it and provide a fix soon.