FreeCAD icon indicating copy to clipboard operation
FreeCAD copied to clipboard

[TechDraw] Clean up precompile in Gui

Open benj5378 opened this issue 3 years ago • 8 comments

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

  • [x] Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
  • [x] In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
  • [x] Your branch is rebased on latest master git pull --rebase upstream master
  • [ ] All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
  • [x] All commit messages are well-written ex: Fixes typo in Draft Move command text
  • [x] Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
  • [x] Commit messages include issue #<id> or fixes #<id> where <id> is the issue ID number from our Issues database in case a particular commit solves or is related to an existing issue. Ex: Draft: fix typos - fixes #4805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.20 Changelog Forum Thread.


benj5378 avatar Aug 04 '22 22:08 benj5378

pipeline status for feature branch PR_7313. Pipeline 605286847 was triggered at b6a2af9. All CI branches and pipelines.

berndhahnebach avatar Aug 04 '22 22:08 berndhahnebach

I could be wrong, but I don't think PreCompiled.h is supposed to appear in *.h files, only *.cpp. If I look at Part/App or Part/Gui, none of the *.h files have it.

WandererFan avatar Aug 05 '22 00:08 WandererFan

pipeline status for feature branch PR_7313. Pipeline 605629230 was triggered at 8a7b2c8. All CI branches and pipelines.

berndhahnebach avatar Aug 05 '22 09:08 berndhahnebach

pipeline status for feature branch PR_7313. Pipeline 605632284 was triggered at 28b8d34. All CI branches and pipelines.

berndhahnebach avatar Aug 05 '22 09:08 berndhahnebach

pipeline status for feature branch PR_7313. Pipeline 606007300 was triggered at 7fcafd6. All CI branches and pipelines.

berndhahnebach avatar Aug 05 '22 16:08 berndhahnebach

pipeline status for feature branch PR_7313. Pipeline 606013582 was triggered at bdd4b36. All CI branches and pipelines.

berndhahnebach avatar Aug 05 '22 16:08 berndhahnebach

This PR is assuming that I understood https://forum.freecadweb.org/viewtopic.php?f=10&t=70796 correctly.

I could be wrong, but I don't think PreCompiled.h is supposed to appear in *.h files, only *.cpp. If I look at Part/App or Part/Gui, none of the *.h files have it.

@wwmayer, do you know anything about this? I'd be very happy if you were able to skim the PR.

I have tested that it compiles on Ubuntu with PCH turned off @The-EG has tested, that it compiles on Windows in Visual Studio with PCH turned on Do we need any additional compiler tests?

benj5378 avatar Aug 05 '22 17:08 benj5378

do you know anything about this? I'd be very happy if you were able to skim the PR.

wandererfan brought up the relevant points. Adding anything about PCH into headers must be avoided. If use a type in a header you can check if a forward declaration is enough but if not then you must include the appropriate header there.

wwmayer avatar Aug 06 '22 17:08 wwmayer

pipeline status for feature branch PR_7313. Pipeline 612824754 was triggered at 2ffe626. All CI branches and pipelines.

berndhahnebach avatar Aug 13 '22 18:08 berndhahnebach

pipeline status for feature branch PR_7313. Pipeline 612901004 was triggered at e3f670d. All CI branches and pipelines.

berndhahnebach avatar Aug 13 '22 23:08 berndhahnebach

pipeline status for feature branch PR_7313. Pipeline 612913987 was triggered at e450300. All CI branches and pipelines.

berndhahnebach avatar Aug 14 '22 00:08 berndhahnebach