Closed adrianinsaval closed 4 years ago
lgtm-com[bot]
commented
4 years ago This pull request introduces 4 alerts and fixes 1 when merging 0a8f28a76ddc363837a447c7b7388def42b720f1 into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
adrianinsaval
commented
4 years ago Can you confirm decodeName is not used? I removed as I didn't find it was used anywhere and it triggered a LGTM warning.
I also removed a couple unnecesary variables from process, generateNaca and generateNacaCoords. do you agree? I'm also thinking we should remove the posX/Y/Z and rotX/Y/Z variables as they don't seem to serve any purpose. I'm guessing the original purpose was to use them when making a wing but I think it's better if that is managed acting directly over the placement of the rib after it is generated. Maybe using expressions. What do you think?
lgtm-com[bot]
commented
4 years ago This pull request introduces 1 alert and fixes 2 when merging f203d8a5b01520d5c2b0f6c67150fbff64d82edf into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
lgtm-com[bot]
commented
4 years ago This pull request introduces 3 alerts and fixes 4 when merging c0015a68d3427e2280016cce0767d611c23444df into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
lgtm-com[bot]
commented
4 years ago This pull request introduces 1 alert and fixes 4 when merging a3f223c329ea42a4f0165a15f81d6dcae90155ae into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
FredsFactory
commented
4 years ago Hello Adrian I am very happy that you wish to contribute to this workshop, and you are welcome, and with great pleasure! I actually have a branch dev where I integrated the HMI in the panels (a standard behavior of object in FreeCAD). In parallel I work on a wrapping of the XFoil lib in order to allow a simulation of the performances of the profiles. Besides your contributions which I really appreciate, could we work on a common roadmap, you have probably lot of ideas to improve my draft?
Fred PS : I'm looking at your pull request asap.
adrianinsaval
commented
4 years ago Yes I would very much like to set common roadmap to work, I sent you PM in the FreeCAD forum with the intention to get in better contact with you for this very purpose. Have you seen my other branch? https://github.com/adrianinsaval/FreeCAD_AirPlaneDesign/commits/devRebaseAndSquash I adapted the changes I made in this PR to your dev branch, still some work to do to with the task panels and possibly other things, I haven't got the chance to properly read what you did yet and I should probably read the freecad documentation on external workbenches too.
I can change the PR to that branch if you want, you may notice that it doesn't have the same commit count from your dev branch, that's because I squashed (see: https://support.gitkraken.com/working-with-commits/squash/) some of your commits to better organize the branch history. For example there was a commit were you accidentally added the pycache files to the repo and afterwards you removed them, by squashing those commits together its as if those files were never uploaded and don't appear in the history.
Another thing I'd like to discuss: it's not clear to me what is your intention with xfoil but I'm not sure if including the makefiles for it in the repo is a good idea. Maybe only the binaries should be included, it probably is necessary to ship binaries for the three OSs: linux, win and mac. Another option would be to just ask the user to install that dependency for the functionality you have planned for it? Or maybe take CFD-Of's approach and fetch the source and build at user request. This is just me brainstorming about it, what do you think?
FredsFactory
commented
4 years ago Ok, I will check your PR branch at the end of week, and if it OK I will contact you to merge the PR and the dev branch (with your help).
lgtm-com[bot]
commented
4 years ago This pull request introduces 1 alert and fixes 4 when merging 03516cdc2acf1f3c8b797c2806436d33b240d1d5 into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
adrianinsaval
commented
4 years ago Ok, I will check your PR branch at the end of week, and if it OK I will contact you to merge the PR and the dev branch (with your help).
Of course! we should coordinate a time to have a chat the weekend
lgtm-com[bot]
commented
4 years ago This pull request introduces 1 alert and fixes 4 when merging 0dbadb3f65290306ecb2d5ccbdc5b44ca2101bad into 7116514e5fcc57f99534a78cccc2f035ea497bfb - view on LGTM.com
new alerts:
fixed alerts:
FredsFactory
commented
4 years ago Global Merge with the help of adriansaval.
Hi Fred! I've been working on this workbench and I think I did some good stuff. I implemented a feature that allows splitting the spline that makes up a wing rib between the upper a lower regions, I think this can be advantageous for better mesh generation when desired.
I also corrected the way lednicer format airfoils (see: https://m-selig.ae.illinois.edu/ads.html) were imported as it didn't play nice when using splines.
Previously if you wanted to force re-import of a .dat file you needed to clear the Coordinates property from python console, now it is auto cleared when you change the path in property editor.
I applied layouts to the current dialog in master to make it resize friendly and I made the preview take the scale of the view rather than base it in chord as it wasn't really meaningful. It can also be zoomed using the mouse wheel now.
I reworked some of the code to make it more readable and efficient and removed many of the unnecessary printing to report view, such behavior is grate for development but not so much for usage.
I also added an icon for the wing rib command and viewprovider, hope you like it.
Finally, I also corrected a typo that was repeated in various places: Desing instead of Design
I'm learning a lot by working on this workbench and would love to continue doing so and help you move this great workbench forward if you want me to. I made this pull request based on the master branch but have later seen your dev branch I rebased your changes and mine to master and sqahes some of your commits to clean he commit history a little, please have a look at it here: https://github.com/adrianinsaval/FreeCAD_AirPlaneDesign/commits/devRebaseAndSquash I can make a PR with that branch instead and discard this one if you want me to.