@mnapoli opened this Issue on October 21st 2014 Member

This is a topic that has been discussed occasionally, let's open a RFC for that topic.

Piwik Core contains many classes that are not related to analytics. For example: event dispatching, MVC, logging, database, IP helpers, URL helpers, caching, filesystem, unzip… (I could go on)

Our options are:

  1. keep them in Piwik Core, in this repository
  2. extract them into reusable, standalone PHP libraries
  3. replace them with existing 3rd parties PHP libraries

Right now, Piwik is using the option 1 except for a few exceptions (Symfony Console, Twig, Zend_Db). I have also applied solution 2 for the new decompress component.

This RFC is about considering options 2 and 3.

This RFC is not about discussing every component and what should we do with it. It's on a more global level: what do you think of clearing up Piwik Core with options 2 and 3, when applicable?

Pros and cons

Removing code from Piwik Core provides the following benefits:

  • if we can reuse a 3rd party library:
    • less code to maintain
    • easier to learn how to use it (for example many developers know how the Twig works)
    • no need to write and maintain documentation
    • we may attract more contributors (or scare less contributors away) with libraries that everybody uses/knows
  • if we extract a standalone reusable component:
    • we could build several high quality "Piwik components" (a bit like Symfony components or the PHP league of extraordinary packages but of course not covering the same things) and have them as a showcase on Piwik's code quality, and ideally attract new contributors and users
    • much easier to test, higher code coverage, better unit testing (so in theory less bugs)
    • higher quality driven by the need of being reusable
    • much easier for contributors (no Piwik to set up, …)
  • in all cases:
    • clearer codebase
    • less tests to run in Piwik Core (tests runtime is a big problem today)
    • easier for new contributors to understand the whole architecture/code organization and contribute

Cons:

  • if we reuse a 3rd party library:
    • we are exposed to potential bugs in that library
    • we might need to contribute to that library with bugfixes
    • we need to follow up on new versions, make sure nothing broke, etc…
    • we might sometimes need to write adapters, i.e. adapt the API of the library to how we want to use it in Piwik
  • if we extract a standalone reusable component:
    • extra work to keep it standalone and reusable
  • in all cases:
    • we have to care about backward compatibility, generally through adapter classes

      When should we use option 1, 2 or 3?

For classes that are not related to Piwik or analytics in general:

  • if we can reuse an existing library that is seriously maintained, then we should reuse it
  • if no 3rd party library does what we want, and if we can provide a better alternative that would be reusable as standalone, then we should open source it as a new standalone component
  • if the classes are too tied to Piwik, then we should keep it in Core

Regarding 3rd party libraries, I would suggest we start with high expectations of quality and reliability. E.g. Symfony components would be OK, we know it is stable, maintained and reliable. For smaller librairies, we have to be careful.

What then?

What do you think of these ideas?

To be clear: this will not be implemented tomorrow. We are just discussing a global direction.

@sgiehl commented on October 21st 2014 Member

+1 for extracting some parts into standalone libraries, like we did for component-decompress or device-detector

@mnapoli commented on October 21st 2014 Member

Ah yes forgot to mention device-detector that's a perfect example! Cool!

@diosmosis commented on October 21st 2014 Member

Switching to 3rd party libs sounds good for smaller bits of code (like say Piwik\Ip or Piwik\Url). Other than making sure they don't change existing behaviour, we should be sure to examine the libraries carefully. They need to be up to our standards (ie, w/ tests, documentation, etc.). Perhaps each potential move to a 3rd party library could be a separate RFC.

For larger chunks of code that could be replaced w/ a 3rd party lib, I think it would be good to move the code to a standalone repository, but have the code only be a well defined interface layer & default implementation that doesn't rely on a 3rd party lib. Then we can adapt any number of libraries for use w/ Piwik, w/o requiring to use them on the wide variety of environments Piwik must run in.

Finally, regarding creating standalone repos: maybe we should have a list of possible refactorings?

@tsteur commented on October 21st 2014 Owner

but have the code only be a well defined interface layer & default implementation that doesn't rely on a 3rd party lib

absolutely!

Would prefer single issues for each "refactoring" rather than a list of possible refactorings.

As @diosmosis said we should do this only for some libraries that do not change often etc. Otherwise we'll have a lot of pain with our submodules and/or we need to look for a different solution for submodules

@mnapoli commented on October 21st 2014 Member

but have the code only be a well defined interface layer & default implementation that doesn't rely on a 3rd party lib

absolutely!

+1 But I don't see why we would need a default implementation if we are using a 3rd party lib. But I definitely agree with having an interface layer (with adapter classes when needed) so that we do not couple Piwik to a specific 3rd party lib.

Perhaps each potential move to a 3rd party library could be a separate RFC.

Would prefer single issues for each "refactoring" rather than a list of possible refactorings.

Definitely agree. I just wanted, before jumping in to specific topics, to make sure we are all on the same page with the global direction. Then we can open issues for each refactoring that we are considering.

we'll have a lot of pain with our submodules and/or we need to look for a different solution for submodules

I don't see how the submodules will be a pain in those cases? We can just install the 3rd party libs with Composer.

@diosmosis commented on October 21st 2014 Member

But I don't see why we would need a default implementation if we are using a 3rd party lib.

Depends on the specific use case, but since Piwik has to run on so many environments it may be the case that it is not possible for some users to use a 3rd party library. Or such users may not want to use the library/technology because it incurs extra work during setup. In such a case, we would have to provide a default implementation that uses base Piwik technologies (ie, PHP + MySQL). An example of this would be parallel job processing. The default implementations would of course be minimalist.

@tsteur commented on October 21st 2014 Owner

I don't see how the submodules will be a pain in those cases? We can just install the 3rd party libs with Composer.

I meant only the own libs we create repositories for. 3rd party libs are not a problem.

Didn't see the part about default implementation. Don't think we'll need a default implementation in all cases either. An interface / bridge is enough. Eg for logs, events, ... we wouldn't have to provide a default implementation but the bridge to implement the interface etc.

@mnapoli commented on October 21st 2014 Member

In such a case, we would have to provide a default implementation that uses base Piwik technologies (ie, PHP + MySQL). An example of this would be parallel job processing. The default implementations would of course be minimalist.

OK I see, I wasn't thinking of such scenarios, it makes total sense. In other simpler scenarios where it's just a simple and plain PHP lib we wouldn't need that though, e.g. what you said @tsteur:

Eg for logs, events, ... we wouldn't have to provide a default implementation but the bridge to implement the interface etc.

@mnapoli commented on October 21st 2014 Member

I meant only the own libs we create repositories for. 3rd party libs are not a problem.

Ah we would use Composer too for our own libs right? E.g. Piwik uses device-detector or the decompress component with Composer: https://github.com/piwik/piwik/blob/master/composer.json#L40-L41

@diosmosis commented on October 21st 2014 Member

In other simpler scenarios where it's just a simple and plain PHP lib we wouldn't need that though, e.g. what you said @tsteur:

This makes sense to me.

@mattab commented on October 22nd 2014 Owner

This will help keep our technical debt low and keep us agile. :+1:

it's a great investment in the future of the platform to create re-usable components. And we should re-use Symfony (or other high quality) components as much as possible, when a similar package is available.

@mnapoli commented on October 22nd 2014 Member

Seems everyone is on the same page! We'll have more targeted issues for specific components/classes, let's close this one.

This Issue was closed on October 22nd 2014
Powered by GitHub Issue Mirror