#552 Added pfunit tool and tests.#562
Conversation
|
There is a certain "asymmetry": this class has a function for getting the include flags, but not the libraries. I would welcome some feedback here. Ideally, we should have a kind of 'dependency' manager, which handles linker AND compiler flags. So far, we only have library flags (in the linker). That would be a bit of work, time we all might not have atm. The consistent solution would be to leave it to the site-specific setup to add the include flags require for pfunit, and I am happy to do that. In lfric, I have a mixin class which automatically adds the include path from pfunit, so it removes the need to add the include flags in all site-specific files (and if a site need something else, they can just add their own pfunit implementation). So, it basically means no need to change any site-specific setup files. But admittedly, it is inconsistent. Not sure what's the best way forward here :) But it is working with skeleton now, so marking this as ready for review. |
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
A question and some food for thought.
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
One minor documentation niggle and I see static type checking and unit testing are failing.
| # which you should have received as part of this distribution | ||
| ############################################################################## | ||
|
|
||
| """This file contains the Rsync class for synchronising file trees. |
There was a problem hiding this comment.
In response to this documentation I say "no it doesn't." Looks like a copy-and-paste mistake.
As discussed, this adds the pfunit tool to Fab.
Note that I have NOT based this on #311 (PR #553). IMHO, the latter is kind of optional. Once we have pFUnit, we don't need really need #311 (it's just a nice to have for the future ;) ).
I am setting this as draft for now, to make sure that the core-fab-with-pfunit branch works as expected.