Helping to review pull requests

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: Helping to review pull requests

Post by ezzieyguywuf »

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
Veteran
Posts: 53930
Joined: Tue Mar 17, 2015 9:14 am

Re: Helping to review pull requests

Post by chrisb »

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.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
ezzieyguywuf
Posts: 656
Joined: Tue May 19, 2015 1:11 am

Re: Helping to review pull requests

Post by ezzieyguywuf »

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: 256
Joined: Sat Sep 07, 2019 6:55 am

Re: Helping to review pull requests

Post by iplayfast »

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.
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: Helping to review pull requests

Post by yorik »

Sorry all, I forgot to look at this thread... Everybody who asked should now have received a github invite and should then be able to help reviewing pull requests.
tomasix
Posts: 1
Joined: Tue Oct 20, 2020 8:30 am

Re: Helping to review pull requests

Post by tomasix »

I have some spare time, guess I can help.


GitHub username: tomasix
User avatar
chennes
Veteran
Posts: 3881
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Helping to review pull requests

Post by chennes »

yorik wrote: Fri Oct 16, 2020 8:28 am Sorry all, I forgot to look at this thread... Everybody who asked should now have received a github invite and should then be able to help reviewing pull requests.
I'd be happy to help, I'm compiling realthunder's Expression refactor right now, and I'm looking forward to giving it a try. chennes on github.

ETA: Disregard... apparently I can review just fine without any special actions on anyone else's part. I don't know if that's a good thing or a bad thing!
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
iplayfast
Posts: 256
Joined: Sat Sep 07, 2019 6:55 am

Re: Helping to review pull requests

Post by iplayfast »

yorik wrote: Fri Oct 16, 2020 8:28 am Sorry all, I forgot to look at this thread... Everybody who asked should now have received a github invite and should then be able to help reviewing pull requests.
Please also add me, I'll help when I can. "iplayfast" on github
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: Helping to review pull requests

Post by yorik »

@tomasix you were there already.
@iplayfast added!
@chennes yes I saw you reviewing... If anyone else can try and confirm, that would be good, then we can close this procedure here. Thanks for all the testing you are doing BTW, that is an incredibly huge help!
User avatar
ebrahim raeyat
Posts: 619
Joined: Sun Sep 09, 2018 7:00 pm
Location: Iran
Contact:

Re: Helping to review pull requests

Post by ebrahim raeyat »

yorik wrote: Mon Dec 14, 2020 12:36 pm @tomasix you were there already.
@iplayfast added!
@chennes yes I saw you reviewing... If anyone else can try and confirm, that would be good, then we can close this procedure here. Thanks for all the testing you are doing BTW, that is an incredibly huge help!
Please add me, I hope i can help the community.
Post Reply