FriendsOfSylius / AwesomeSylius

Welcome to the SyliusGoose, which is the place you will find more information about Sylius related things.
110 stars 20 forks source link

RFC - Contribution rules #10

Closed gabiudrescu closed 6 years ago

gabiudrescu commented 6 years ago

While writting https://github.com/friendsofsylius/SyliusGoose/issues/9 I have realised that we lack a contribution guideline and a public set of acceptance criteria for plugins to reach this list.

For example, what criteria should a plugin / bundle follow to be added here? Should it be 1.0 compatible? Should it have test coverage?

Let's have a discussion about this and define a contribution guideline for this repo.

bitbager commented 6 years ago

This is something we already discussed in #6. Because more and more people are creating new plugins, I think it's the right time to take care of it.

For me, these are the basic requirements for Sylius plugin:

  1. It must be covered at least with PHPSpec, perfectly with PHPSpec, Behat and PHPUnit (if necessary).
  2. It must have a description with an overview, installation guide, and contribution guide.
  3. It must be compatible with Sylius 1.0.0+
  4. It must be tagged with a version
  5. If it's based on SyliusPluginSkeleton, it must be named plugin. If not, it could be named bundle.
  6. Obviously, it must follow composer standards and be published on packagist.
  7. It must follow Sylius coding standards.

What do you think?

stefandoorn commented 6 years ago

I would adjust 1. to something like: It must be covered by tests. Not specific to PHPSpec imo. Not everyone is very comfortable with it, but might be very comfortable using PHPUnit covering all classes.

pamil commented 6 years ago

I'd even put Behat over PHPSpec tests. Acceptance testing is already set up in the skeleton and makes sure that plugin is wired up correctly and nothing is conflicting.

Behat steps aren't backwards compatible yet, but there shouldn't be too much changes (and I'd like to put them under BC promise with 1.1 release).

bitbager commented 6 years ago

@pamil, of course, but sometimes you only want a package to have a simple code improvement, for instance, a Twig extension that supports assets versioning. I think we will need to specify which testing tools should be used for which code case.

bitbager commented 6 years ago

@stefandoorn, I agree, but I think that diving into it is not hard. Sylius has made a lot of work to make it possible and not using it for Sylius plugins is a little bit like a punch in the face IMHO 🙁

stefandoorn commented 6 years ago

True, but I think there should be tests for the right purpose over naming certain tools. I'm not sure e.g. if my sitemap plugin tests differ that much through phpunit or behat at the moment, e.g. I agree that with front-end changes Behat would be preferred and it's rather easy to start with as the skeleton provides help. Else it's rather tedious to setup yourselves I figured out.

Still need to figure out how to test javascript variables with Behat, as I need that.

pamil commented 6 years ago

@stefandoorn what do you mean by testing javascript variables?

stefandoorn commented 6 years ago

Actually how to use Behat to check whether certain JS variables exist / have a certain value, on global level. I mainly see examples about using css selectors to fetch elements, but never figured out if it's possible to test against JS variables in the HTML.

stefandoorn commented 6 years ago

@pamil Something like this idea: https://gist.github.com/jhedstrom/7017508. Want to test what is in certain JS variables injected via Twig views - to make sure the correct data will be loaded to e.g. Google Tag Manager.

bitbager commented 6 years ago

Closing due to the updated PLUGIN_STANDARDS.md file.