[Form] Add cookbook entry how to code bundle that will be compatible with 2.3 and 2.8 Symfony versions.

[Koc]

Third party bundles should invent something like Sf2CompatUtil classes see https://github.com/symfony-cmf/RoutingBundle/pull/325/files . I think that should be official recommendation from core team how to deal in this case.

[Koc]

// cc @webmozart

[Koc]

relates but not fixed in #5894

[greg0ire]

Indeed not fixed. It would be great to have a small library that is able to do feature detection outside symfony. Something like symfony/feature-detection . It could provide a helper with methods like formComponentSupportsChoicesAsValues(), and another helper that could provide the right arguments for a given method depending on what is detected (for instance, it would flip an array of choices only when appropriate).

[greg0ire]

Here is the design I would use to avoid ending with a big god object :

/**
 * Interface to implement for new detectors
 */
interface FeatureDetectorInterface
{
    /** @return string the name of the feature, for instance form/choices-as-values */
    public function getName();

    /** @return boolean */
    public function isFeatureSupported();
}

/**
 * Public API of the lib
 */
interface FeatureDetectorRegistryInterface
{
    /**
     * @param string
     * @return boolean
     */
    public static function isSupported($feature);
}

/**
 * Example implementation
 */
class FeatureDetector implements FeatureDetectorRegistryInterface
{
    public static function isSupported($feature)
    {
        return self::registry[$feature]->isFeatureSupported();
    }
    /**
     * Something could iterate on a directory and call this for each class found in it
     * @param FeatureDetectorInterface
     */
    public static function register(FeatureDetectorInterface $featureDetector)
    {
        self::registry[$featureDetector->getName()] = $featureDetector;
    }
}

@Koc @andrewtch what do you think ?

[andrewtch]

It thing this is very, very, very bad idea. Instead of relying on versions and interfaces, we rely on some voodoo and a lot of hardcoded strings. This approach must never be used.

[greg0ire]

It thing this is very, very, very bad idea

I agree with you, it is very ugly and a bit sad to have that. It would be better if each component had its own version from the start. But it does not, and obviously we cannot add it afterwards. So what can we do? Not relying on the kernel version is (or at least, will soon be) the way to go, see for yourself : https://github.com/symfony/symfony-docs/pull/5894/files#diff-85d0e4541aa9fa8719abcc50a45c7382R168

[andrewtch]

It's stated clearly - rely on features, not hardcode.

[greg0ire]

It's stated clearly - rely on features, not hardcode.

What does it mean ? I don't understand, can you give an example ?

[greg0ire]

@andrewtch ? @andrewtch ? Oh he flew away, leaving us with this enigmatic sentence :

rely on features, not hardcode

What did he mean with "hardcode" ? form/choices-as-values for instance ? What solution did he have in mind when he decided to leave us forever ? I guess we'll never know…

[soullivaneuh]

On thing I'm afraid with @greg0ire proposition is the usage of an interface to detect code compatibility.

But, if this interface has to be changed for some reason with some BC break and/or deprecated stuff, we will have to manually detect if the compatibility class is compatible before check the compatibility of the method.

Compaception. That would be overkill.

Or, I did not understand at all the purpose of this interface. :-)

[greg0ire]

The interface should be deliberately kept very simple and stable, to avoid Compaception . That's why having one method isSupported felt good to me.

[andrewtch]

@greg0ire , that was rude.

Symfony version ir Hardcoded. Having God object is also hardcoded, we should avoid it.

Checking for interfaces is fine. Making isSupported, again, is not - by the Inversion-Of-Control pattern one should use "supports" method, like "ChoiceList::supportsPlaceholder".

[greg0ire]

@greg0ire , that was rude.

Well saying things are very very bad and then runaway with no explanation is not precisely what I'd call polite if you ask me…

Making isSupported, again, is not - by the Inversion-Of-Control pattern one should use "supports" method, like "ChoiceList::supportsPlaceholder".

I know, but you can only add this method afterwards. So you will not have it. That's why you are obliged to have this call to an external lib / symfony component. Because you cannot rewrite the past. And BTW, who would write ChoiceList::supportsPlaceholder and not implement placeholder in ChoiceList ?

Or maybe you are advocating ChoiceList in another namespace / a separate component ?

It's stated clearly

It's actually very hard to get you to state things clearly, but I feel it is started to come. Please take my sarcasms / jokes less seriously, and speak your mind, maybe something good will come out of this.

[andrewtch]

For the topic and 2.3 vs 2.8:

• use version_compare where possible (instead of example given)
• create and support different bundle versions since 2.0, 2.3 and 2.6+ are basically different symfony versions

In all cases the later approach (with different branches, like master in sync with latest and per-release branches) gives you cleaner code.

What you offer is to change one if - hell in bundle like:

if ($version == 2.2) { 
   ...
} elseif ($version == 2.3) {
    ...
} elseif ($version == 2.4) {
    ...
} elseif ($version == 2.5) {
    ...
}
///etc

with another if-hell like

if (Detector::supports('2_2_feature') { 
   ...
} elseif (Detector::supports('2_3_feature') {
    ...
} elseif (Detector::supports('2_4_feature') {
    ...
} elseif (Detector::supports('2_5_feature') {
    ...
}
///etc

Both approaches are bad and leave a lot of legacy code.

So: many symfonies -> many bundle versions

[wouterj]

Please, we are all volunteers. Please be patient if someone doens't respond 24/7.

@andrewtch the problem with checking by versions is that it's not possible to get the current version. There only is versions on the HttpKernel component. But you can install Form 2.3 with HttpKernel 2.8 for instance, so relying on this version is not an option.

Instead, checking for some interfaces or methods of which you know are introduced in a specific minor release is the way to go. I think grouping them in a simple method (as I tried to do for the CmfRoutingBundle) will make things a lot easier. This doesn't need an interface, as it'll just be static calls anyway.

Oh, btw. Maintaining many versions of the same bundle is undoable from a bundle maintainers point of view.

[greg0ire]

Ok so what you're stating is : a (major) version of the bundle should support one and only one major version of symfony (and of all of its dependencies).

The topic is "how do you support several properly ?", and I had a hard time figure out you actually mean't "just don't". I thought you had some other solution, that's why I was asking for explanations. The thing is, I'm not the one making this call. I'm in the situation where I must support several versions with one version of the bundle.

[greg0ire]

This doesn't need an interface, as it'll just be static calls anyway.

I chose to present them in an interface because I wanted us to discuss about the interface of the implementation, but you're right, FeatureDetectorRegistryInterface won't be actually needed. FeatureDetectorInterface will be useful, but for people who want to add new detections.

[javiereguiluz]

Sadly I must close this as "won't fix". We simply were too late for this. Symfony 2.3 ended its support in May 2016 and bug fixes will be fixed only until the end of May 2017. Thanks for understanding it.