cgreen-devs
Cleanup and change constraint names
In this pull request I am doing the following changes:
- Move
constraint.htointernal/constraint.h(Fixes #52) - Rename
internal/constraint.htointernal/constraints.h(Fixes #52) - Rename the
Constraintstruct toCGreenConstraint
The first 2 are part of the issue #52 the last one was something that we observed while using CGreen. The struct Constraint can already be defined in the users application which will cause an error while compiling the test.
To solve this issue the approach was to prefix the structure name with CGreen
Coverage remained the same at 96.357% when pulling 49e08d36f5f0ba1cb985e5f221e11ac0f7040cde on joaopapereira:cleanup-change-constraint-names into 5c078a6fd72bec1c1cdb81fce0ecf1b5b569ecc8 on cgreen-devs:master.
True. We have some problems with the Shippable configuration, and I've been meaning to remove it.
I think @matthargett 's comment need to be considered. We don't want to limit users possibility to extend Cgreens functionality. I did not consider this when I wrote the issue because I had not seen that used anywhere.
Although you would still find the include file and use that, the "internal" part signals that it is not intended for external use, so if that scenario is to be supported, we should keep it where it is.
As a sidenote I think it would be very valuable to have a section in the manual about extending Cgreen with custom constraints. That also signals that this is an intended API. (Think there are also some "API-checking type tests" somewhere which should include this.)
Otherwise I think the name change is a worthwhile change, and also probably necessary if this should be API.
So I suggest to keep the name change of Constraint, but keep constraint.h in the public/API directory.
If constraint_syntax_helper.h was renamed to constraints.h (note the plural) I think that would address the core of the intent of #52, namely that a "normal" user would include constraints.h and that would be all that would be needed.
If you would implement special constraints, you might also need to include constraint.h.
I think @thoni56 and @matthargett are right moving constraint.h into internal is hiding the fact that we can create new constraints.
So I will implement the suggestion from @thoni56 and will rename constraint_syntax_helper.h to constraints.h
My only concern would be the name proximity between constraint and constraints but I think we can use the documentation from the story that you created to help better understand each file purpose.
Also noticed that my patch had an inconsistency with names. In the code we use Cgreen.. for Cgreen related types, but my patch was using CGreen so I changed that.