Add ObjectTypeConfig and ArgConfig

[mcg-web]

Introduce optional ConfigBuilder (see #70)

[coveralls]

Coverage decreased (-0.1%) to 88.257% when pulling 0fc1759 on mcg-web:type-config-builder into 986eff9 on webonyx:master.

[mcg-web]

@vladar can you review this please? if OK for you i'll add tests to cover remaining code

[coveralls]

Coverage increased (+0.06%) to 88.409% when pulling 95b46b7 on mcg-web:type-config-builder into 986eff9 on webonyx:master.

[vladar]

Hey by the way - will be very glad to see this feature in our lib. Thanks for this work!

[vladar]

I just realized that we must also cover case when fields and interfaces are defined via Closure. I guess addField is not enough here. I only see following as a solution, but maybe you have other ideas:

$config = ObjectTypeConfig::create()
  ->name('Test')
  ->fields(function() {
    return FieldsConfig::create()
      ->addField('name', Type::string(), null, null, ArgsConfig::create()
        ->addArg('arg1')
        ->addArg('arg2')
      )
    );
  );

Then we can re-use FieldsConfig for interface type

Edit: another possible approach:

$config = ObjectTypeConfig::create()
  ->name('Test')
  ->fields(function(ObjectTypeConfig $conf) {
    return $conf
      ->addField('name', Type::string(), null, null, ArgsConfig::create()
        ->addArg('arg1')
        ->addArg('arg2')
      )
    );
  );

Not sure what complications it might cause though.

[vladar]

Another thing to consider is if we should put resolve as 3rd argument. The reason is that we also have resolveField callback in type config which makes resolve unnecessary (I personally prefer to use it vs resolve).

Do you have any objections if we put $resolve after $args so that all definitions go first and callbacks after them?

[vladar]

I would also suggest to introduce addDeprecatedField() to make it visually different from regular fields. Then we'll have two methods:

addField($name, $type, $description = null, ArgsConfig $args = null, callable $resolve = null, callable $complexity = null)
addDeprecatedField($name, $type, $description = null, ArgsConfig $args = null, $deprecationReason, callable $resolve = null, callable $complexity = null)

Please let me know if it makes sense for you.

[vladar]

And yet another idea is to have some facade class which will allow us to avoid remembering ObjectTypeConfig, FieldsConfig, ArgsConfig, etc.

Maybe some Config::objectType(), Config::fields(), Config::args() etc. Then we only need to use one class and can rely on autocomplete for different config sections. If it makes sense.

[mcg-web]

ok no prob for the order of the addField Arguments, also +1 for the addDeprecatedField. we must be careful of reusing fields definition in closure because same instance will be share in interface and object. I'll work on all this latter. Thanks for the review :)

[coveralls]

Coverage decreased (-0.3%) to 88.068% when pulling 31cdc0b on mcg-web:type-config-builder into 986eff9 on webonyx:master.

[vladar]

Feel free to merge it yourself when ready.

One thing that is missing though is documentation. New docs are stored in /docs folder. Please add new file about builder when you have a chance. I guess it makes sense to explicitly mention that it is experimental API which will likely to change (docs are powered by mkdocs, so documentation site structure is defined in mkdocs.yml).

Also just a friendly request - can you avoid Traits? I know it is opinionated view, but if we can avoid them without major impact on codebase - I'd appreciate it. If not - I can still live with it.

[mcg-web]

Ok i'll work on this later 👍

[enumag]

Can we get this merged? Even without documentation the builders are quite clear from the testcase and I'd like to use them.

[mcg-web]

I completely forgot this PR, I will add documentation this week end so we can merge it 👍

[enumag]

@mcg-web Awesome! 👍

[coveralls]

Coverage decreased (-0.1%) to 92.745% when pulling 758294a on mcg-web:type-config-builder into a3f18b5 on webonyx:master.

[enumag]

So, any progress? :-)

[mcg-web]

I'm Improving the coverage to be sure to don't miss nothing... I'll submit my changes this morning.

[mcg-web]

@vladar this can be merge in master or we must wait for next major release?

[enumag]

Please wait for a bit. I dug a bit deeper into it and I think we might want to use a different approach. I'm writing an issue right now.

[coveralls]

Coverage increased (+0.07%) to 92.939% when pulling 691f558 on mcg-web:type-config-builder into a3f18b5 on webonyx:master.

[mcg-web]

OK here is the initial issue #70

[vladar]

I think I can merge it before the next version release. So just keep it as is. Actually, I am not sure if we do need it at all (since many people introduce their own methods of building types or use framework-specific libs which do it for them).

Adding it to the main library may add some confusion about the right way to do things. @mcg-web what do you think about it? Will you guys use these builders for symfony bundle?

[mcg-web]

No in the bundle we using the generator types so no need of this. We can close this. Less codes to maintain :D.