phpList/core

Project Structure Proposal

kilip opened this issue · 4 comments

kilip commented

Namespace Refactoring

Currently we include version in our namespace:

namespace PhpList\PhpList4\Core

I think we will have trouble when relasing a next major version for example 5.0.0. Contributor will get confused whether the same class can be used or not in 5.0.0 version. Please take a look at symfony. Symfony have a different version from 2.0 to 4.0. Some components act differently in each version. To make maintenance easier we don't see any version included on symfony namespace.

We can use git branch feature to maintenance spesific version of phplist, so we don't have to stick a namespace into a spesific namespace. For example:

  • branch 3.0: for phplist version 3.0 (yes, we can even use same git repository to maintenance 3.0 version)
  • branch 4.0: for phplist version 4.0

Thanks to psr4 autoloading. This namespace will create an unnecessary sub directory in psr0 autoload:

// path/to/phplist4-core
src/
----- PhpList
----------- PhpList4
------------------ Core

I suggest we just use a more simple also a more common practice used by an opensource project:

namespace PHPList\Core;

I know this refactoring can produce a lot of error. With find and replace (php editor feature), and phpunit tests to detect any error; I think we can refactor namespace easily.

Testing Configuration

1. phpunit.xml.dist File

By default phpunit will search phpunit.xml in the current working directory (path/to/phplist4-core), if this file is not exists then phpunit try to find default configuration file which is phpunit.xml.dist. I think it's more simple to just use phpunit without having to pass --configuration in command:

# path/to/phplist4-core

./vendor/bin/phpunit
# command above is more simple then:
./vendor/bin/phpunit  -c Configuration/PHPUnit/phpunit.xml

So I suggest we need to put our configuration file in path/to/phplist4-core/phpunit.xml.dist, and git ignore the path/to/phplist4-core/phpunit.xml file because it should be a custom configuration for every developer.

2. Testing Configuration

We should separate testing configuration from project configuration. so we need to move all tests configuration from Configuration directory to tests/config.

Directory Naming Conventions

I suggest that we use lowercase for non namespace directory. So when we see Configuration directory we will know that it will contain a class, while config dir only contain a configuration file. Here's my sugestion:

  • tests directory: Since there are no namespace called Integration, Support, and Unit, we just use lowercase name for this directories
  • Configuration directory: we just use simply use config because it is a common name used by symfony based project to store configuration files.
  • Database directory: we need to rename this directory to data.

Here's what I hope phplist4-core project directory like:

path/to/phplist4-core
├── config
│   ├── config.yml
│   └── # other config files...
├── data
│   ├── phplist-db.mwb
│   └── Schema.sql
├── src
│   ├── Core
│   ├── Routing
│   └── System
└── tests
    ├── config
    │   ├── code-sniffer-ruleset.xml
    │   ├── phpmd-ruleset.xml
    │   └── phpunit-travis.xml
    ├── integration
    │   ├── Composer
    │   └── Core
    ├── system
    └── unit
        ├── Core
        └── Domain

Some References

Here's some sample of good symfony based project that we can implement in phplist:

Hi @kilip, thanks for the discussion points.

I'm fine with changing to namespace PHPList\Core; (or namespace PhpList\Core;, which I prefer so we have real CamelCase). We should also change the package name to just phplist/core and also rename the GitHub repository to core.

As for the PHPUnit configuration: In the long term, we might need multiple configuration files anyway. So I'd prefer to keep them in a directory. We'll shorten the call by adding a composer script.

Concerning the directory naming conventions: We're already working on this. Within tests/ and src/, the directories cannot be all-lowercase due to the PSR-4 autoloading.

@samtuke, @michield What do you think?

Yes, fine by me. phplist/core is more obvious. Eventually the version is irrelevant. We called it phpList4 to distinguish it. I like that better than things like "-new" as after some time it won't be new any longer.

There will however, for the time being, be some old and new aspect of the two phpLists, with eg the sending engine being in pl3. But eventually, the plan is to move away from that.

Camelcase wise, I really like "phpList" more than any other version. It's quite distinct.

I have changed the repository and Packagist name to 'core' following agreement above and am working on the composer.json changes in #275

Regarding directory naming conventions: all good that this is already agreed and in progress