Helping to review pull requests

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: Helping to review pull requests

Postby ezzieyguywuf » Wed Aug 26, 2020 6:59 pm

chrisb wrote:
Wed Aug 19, 2020 11:45 pm
Thank you!
No problem.

By the way, I haven't forgotten about this, just trying to find the best way to tackle it, as it's a big PR.

Also, I have strong opinions regarding coding styles/techniques that generally don't seem to align with RealThunder's approach.

I think my first step will be to take a small portion of the PR, and post my ideas here on the forum somewhere in order to start a discussion, as I want to be sure that the time I spend reviewing is put to good use.
chrisb
Posts: 27297
Joined: Tue Mar 17, 2015 9:14 am

Re: Helping to review pull requests

Postby chrisb » Wed Aug 26, 2020 9:17 pm

ezzieyguywuf wrote:
Wed Aug 26, 2020 6:59 pm
By the way, I haven't forgotten about this, just trying to find the best way to tackle it, as it's a big PR.

Also, I have strong opinions regarding coding styles/techniques that generally don't seem to align with RealThunder's approach.
Perhaps it's due to these facts that it isn't merged yet. I'm sure your comments will be welcome.
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: Helping to review pull requests

Postby ezzieyguywuf » Wed Aug 26, 2020 10:13 pm

chrisb wrote:
Wed Aug 26, 2020 9:17 pm
Perhaps it's due to these facts that it isn't merged yet. I'm sure your comments will be welcome.
I was hoping for this sort of response!
User avatar
iplayfast
Posts: 135
Joined: Sat Sep 07, 2019 6:55 am

Re: Helping to review pull requests

Postby iplayfast » Thu Aug 27, 2020 11:09 pm

I've been testing realthunder's LinkStage3 code and I've got to say, its fixed some of the problems I was having regarding references. Also fixed some crashes I was having, but I'm not sure if that's the master's regressions or realthunder's fixes.
https://forum.freecadweb.org/viewtopic. ... 5&start=10 It doesn't fix all, but definitely enough to try to get into .19 (IMHO).

If it's code format that is the issue in pulling the merge, that can be changed after the merge. Does the FreeCad standard not have a tool to do this reformatting automatically? If not, let's change the "standard" to something more standard. (meant to be humorous, not aggressive).

The techniques you don't like, can you describe in general what they are, or point to an area in the source code that show it? I'd like to help, but not sure how much time I can commit, but at least a few hours a week.