[WIP] Configuration system refactor #527

Merged
merged 3 commits into from Jul 15, 2014

Projects

None yet

7 participants

@cristianoc72
Propel member

See issue #285.

Basic ideas

  • Reorganize the configuration properties in a more rational hierarchy
  • Support for multiple formats
  • Validate configuration

Implementation details

Some of the following points should be discussed.

  • Based on Symfony Config Component
  • Support for .php, .ini, .json, .yaml, .xml formats.
    • Ini file loader accepts both .ini and .properties files.
    • Inside ini file, we can define a hierarchy in standard way (with sections and array definitions) or in a "nested" way, inspired by Zend Framenwork Config.
  • Ability to define some parameters, in any configuration format, delimited by % (see Symfony\Component\DependencyInjection\ParameterBag class). Example:
# yaml
database:
    adapter: mysql
    host: localhost
    dsn: %adapter%:host=%host%

becomes:

// php
array(
    'database' => array(
        'adapter' => 'mysql',
        'host' => 'localhost',
        'dsn' => 'mysql:host=localhost'
    )   
);
  • I've put all new classes in Propel\Common\Config namespace. I've choosen Propel\Common because, theoretically, configuration system can be used both at generation and runtime, even if, for the most part, is used at generation time.
  • I supposed to define a single configuration file, named propel-config.ini propel.ini (or .yaml or some other supported format) which should be located in project root directory or in a sub-directory named "conf" or "config" (we can choose something else). This file also replace runtime.conf and buildtime.conf.
  • Default configuration, shipped with Propel, is placed in tools/Generator/default-config.yaml file (may be it could be better tools/Config/default-config.yaml). Default properties can be override by project configuration, which can be override by options passed to command line. Moved to step 2

Roadmap

  • Step 1: add new classes and relative tests
  • Step 2: move all default options from command classes to default-conf.yaml and implement Propel\Common\Config\PropelConfiguration class to validate configuration and load defaults, via TreeBuilder. Remove all not more used options and define a better property structure.
  • Step 3: refactor all those classes using configuration.
  • Step 4: update documentation

Each step can be merged separately.

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/ConfigurationManager.php
+ $this->load();
+ $this->process();
+ }
+
+ /**
+ * Return the whole configuration array
+ *
+ * @return array
+ */
+ public function get()
+ {
+ return $this->config;
+ }
+
+ /**
+ * Return a specific session of the configuration array.
@staabm
staabm Jan 17, 2014

what is a session of configuration? do you mean section?

@cristianoc72
cristianoc72 Jan 18, 2014

umpf....it's section. My mid-school level English....

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/ConfigurationManager.php
+ $dirs[] = $currentDir;
+ if (is_dir($currentDir . '/conf')) {
+ $dirs[] = $currentDir . '/conf';
+ }
+ if (is_dir($currentDir . '/config')) {
+ $dirs[] = $currentDir . '/config';
+ }
+
+ $finder = new Finder();
+ $finder->in($dirs)->files()->name($this->fileName . '.*');
+
+ if (count($finder) <= 0) {
+ throw new InvalidArgumentException('Propel configuration file not found');
+ }
+
+ $files = iterator_to_array($finder);
@staabm
staabm Jan 17, 2014

you could just use current($finder)?

@staabm staabm commented on the diff Jan 17, 2014
...Propel/Common/Config/Exception/JsonParseException.php
+ * @license MIT License
+ */
+
+namespace Propel\Common\Config\Exception;
+
+class JsonParseException extends RuntimeException implements ExceptionInterface
+{
+ /**
+ * Create an exception based on error codes returned by json_last_error function
+ *
+ * @param array $errors Array of LibXMLError objects
+ * @see http://www.php.net/manual/en/function.json-last-error.php
+ */
+ public function __construct($error)
+ {
+ $message = 'Error while parsing Json configuration file: ';
@staabm
staabm Jan 17, 2014

should this be special cased, because since php 5.5 this is available builtin?
http://php.net/json_last_error_msg

not important though.

@cristianoc72
cristianoc72 Jan 18, 2014

Yep. May be I could refactor it to use json_last_error_msg for php >=5.5

if (function_exists('json_last_error_msg') {
     //use json_last_error_msg
} else {
   //current implementation
}

Do you think it could improve performance?

@staabm
staabm Jan 18, 2014

Since this is a exception case it won't be a perf gain. But in case php will add more error levels they would immediately availabe when unsing the builtin func.

@staabm staabm commented on the diff Jan 17, 2014
src/Propel/Common/Config/Exception/RuntimeException.php
@@ -0,0 +1,15 @@
+<?php
+
+/**
+ * This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @license MIT License
+ */
+
+namespace Propel\Common\Config\Exception;
+
+class RuntimeException extends \RuntimeException implements ExceptionInterface
@staabm
staabm Jan 17, 2014

I think we should use a different name, otherwise it can easily occur that one misses a namespace and therefore uses wrong exception types..

@staabm
staabm Jan 17, 2014

not important..

@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Exception/XmlParseException.php
+namespace Propel\Common\Config\Exception;
+
+class XmlParseException extends RuntimeException implements ExceptionInterface
+{
+ /**
+ * Create an exception based on LibXMLError objects
+ *
+ * @param array $errors Array of LibXMLError objects
+ * @see http://www.php.net/manual/en/class.libxmlerror.php
+ */
+ public function __construct(array $errors)
+ {
+ $numErrors = count($errors);
+
+ if (1 == $numErrors) {
+ $message = "An error occurs ";
@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Exception/XmlParseException.php
+class XmlParseException extends RuntimeException implements ExceptionInterface
+{
+ /**
+ * Create an exception based on LibXMLError objects
+ *
+ * @param array $errors Array of LibXMLError objects
+ * @see http://www.php.net/manual/en/class.libxmlerror.php
+ */
+ public function __construct(array $errors)
+ {
+ $numErrors = count($errors);
+
+ if (1 == $numErrors) {
+ $message = "An error occurs ";
+ } elseif ($numErrors >1) {
+ $message = "Some errors occur ";
@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/ConfigurationManager.php
+ if (!array_key_exists($session, $this->config)) {
+ return null;
+ }
+
+ return $this->config[$session];
+ }
+
+ /**
+ * Find a configuration file and loads it.
+ * Configuration file is expected to be found in the current directory
+ * or a subdirectory named '/conf' or '/config'.
+ * Only one configuration file is supposed to be found.
+ */
+ private function load()
+ {
+ $currentDir = getcwd();
@cristianoc72
cristianoc72 Jan 18, 2014

Nope. File locator pretends a file name with extension. I can't pass something like propel-config.* because it raises an exception for non existent file.

@staabm staabm commented on the diff Jan 17, 2014
src/Propel/Common/Config/Exception/XmlParseException.php
+ }
+ $message .= "while parsing XML configuration file:\n";
+
+ foreach ($errors as $error) {
+ $message .= " - ";
+
+ switch ($error->level) {
+ case LIBXML_ERR_WARNING:
+ $message .= "Warning $error->code: ";
+ break;
+ case LIBXML_ERR_ERROR:
+ $message .= "Error $error->code: ";
+ break;
+ case LIBXML_ERR_FATAL:
+ $message .= "Fatal Error $error->code: ";
+ break;
@staabm
staabm Jan 17, 2014

do we need a default case here?

@cristianoc72
cristianoc72 Jan 18, 2014

Well, I thought about it, but the documentation says that level property can assume only those three values http://www.php.net/manual/en/class.libxmlerror.php

@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/DelegatingLoader.php
+
+namespace Propel\Common\Config\Loader;
+
+use Symfony\Component\Config\Loader\DelegatingLoader as BaseDelegatingLoader;
+
+/**
+ * Class DelegatingLoader
+ *
+ * @author Cristiano Cinotti
+ */
+class DelegatingLoader extends BaseDelegatingLoader
+{
+ /**
+ * @param array $loaders
+ */
+ public function __construct(array $loaders = null)
@staabm
staabm Jan 17, 2014

param not used

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/FileLoader.php
+ *
+ * @author Cristiano Cinotti
+ */
+abstract class FileLoader extends BaseFileLoader
+{
+ /**
+ * Propel default configuration file
+ *
+ * @var string
+ */
+ private $defaultFile;
+
+ /**
+ * Instance of Yaml parser
+ *
+ * @var object|Symfony\Component\Yaml\Parser;
@staabm
staabm Jan 17, 2014

why object|?

@cristianoc72
cristianoc72 Jan 18, 2014

Ide error I didn't see

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/FileLoader.php
+
+ if (null === $locator) {
+ $locator = new FileLocator();
+ }
+
+ parent::__construct($locator);
+ }
+
+ /**
+ * Merge Propel default configuration values into $configuration array
+ *
+ * @param array $configuration The configuration array
+ *
+ * @return array
+ */
+ public function mergeDefaults($configuration)
@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/FileLoader.php
+ *
+ * @return array
+ */
+ public function mergeDefaults($configuration)
+ {
+ $defaults = $this->getYamlParser()->parse(file_get_contents($this->defaultFile));
+
+ return array_merge($defaults, $configuration);
+ }
+
+ /**
+ * Replaces parameter placeholders (%name%) by their values for all parameters.
+ *
+ * @param array $configuration The configuration array to resolve
+ */
+ public function resolve($configuration)
@staabm
staabm Jan 17, 2014

array typehint?

@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/FileLoader.php
+ $parameters = array();
+ foreach ($configuration as $key => $value) {
+ $key = $this->resolveValue($key);
+ $value = $this->resolveValue($value);
+ $parameters[$key] = $this->unescapeValue($value);
+ }
+
+ $this->resolved = true;
+
+ return $parameters;
+ }
+
+ /**
+ * Get an instance of Yaml parser.
+ *
+ * @return object|Symfony\Component\Yaml\Parser
@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/FileLoader.php
+ $found = false;
+
+ $ret = $this->getValue($property_key, null, $found);
+
+ if (false === $found) {
+ throw new InvalidArgumentException("Parameter '$property_key' not found in configuration file.");
+ }
+
+ return $ret;
+ }
+
+ /**
+ * Scan recursively an array to find a value of a given key.
+ *
+ * @param $property_key The array key
+ * @param $config The array to scan
@staabm
staabm Jan 17, 2014

missing $found

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/JsonFileLoader.php
+{
+ /**
+ * Loads an Json file.
+ *
+ * @param mixed $file The resource
+ * @param string $type The resource type
+ *
+ * @throws \InvalidArgumentException if configuration file not found
+ * @throws Propel\Common\Config\Exception\InvalidArgumentException if invalid json file
+ */
+ public function load($file, $type = null)
+ {
+ $path = $this->locator->locate($file);
+ $content = array();
+
+ $json = file_get_contents($path);
@staabm
staabm Jan 17, 2014

should we handle file permission checks in the loaded classes?

@cristianoc72
cristianoc72 Jan 20, 2014

You're right. I run a test case about it and all the loaders failed. I refactored the code to throw an exception when the file isn't readable.

@staabm staabm and 1 other commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/Loader/JsonFileLoader.php
+ * @throws Propel\Common\Config\Exception\InvalidArgumentException if invalid json file
+ */
+ public function load($file, $type = null)
+ {
+ $path = $this->locator->locate($file);
+ $content = array();
+
+ $json = file_get_contents($path);
+
+ if (false === $json) {
+ throw new InvalidArgumentException('Error while reading configuration file');
+ }
+
+ if ('' !== $json) {
+ //json_decode function needs an utf8 string
+ $json = utf8_encode($json);
@staabm
staabm Jan 17, 2014

why do you assume that the file is iso 8859 encoded?

@cristianoc72
cristianoc72 Jan 18, 2014

Oh, you're right. I misunderstood documentation!

@staabm staabm commented on the diff Jan 17, 2014
src/Propel/Common/Config/Loader/LoaderResolver.php
+
+/**
+ * Class LoaderResolver
+ *
+ * @author Cristiano Cinotti
+ */
+class LoaderResolver extends BaseLoaderResolver
+{
+ /**
+ * @param array $loaders
+ */
+ public function __construct(array $loaders = null)
+ {
+ if (null === $loaders) {
+ $loaders = array(
+ new IniFileLoader(),
@staabm
staabm Jan 17, 2014

no sure about the order here... i think we should order it from the most common used to the least common, or we should state that mixing formats is discouraged

@cristianoc72
cristianoc72 Jan 26, 2014

Since we support many formats, we don't know which loader will be used a priori. This is the reason why we load each one in DelegatingLoader class and leave this class to decide. Imho there's no way to know which is the most used format: I like Yaml and I use it whenever it's possible but I don't know about other people.

@staabm staabm commented on an outdated diff Jan 17, 2014
src/Propel/Common/Config/XmlToArrayConverter.php
+ //Empty xml file returns empty array
+ if ('' === $xmlToParse) {
+ return array();
+ }
+
+ if ($xmlToParse[0] !== '<') {
+ throw new InvalidArgumentException('Invalid xml content');
+ }
+
+ libxml_use_internal_errors(true);
+
+ $xml = simplexml_load_string($xmlToParse);
+ $errors = libxml_get_errors();
+
+ libxml_clear_errors();
+ libxml_use_internal_errors(false);
@staabm
staabm Jan 17, 2014

need to be reset to initial state, not hardcoded to false

@staabm
Propel member

@cristianoc72 nice job man, looks cool.

when reading all those format related things, I wonder if we couldn't re-use some symfony components because symfony also supports most of this formats...? not sure if we re-invent the wheel as far as config format handling is concerned.

@willdurand
Propel member

Nice!

I would like to get rid of the default files. Add default values to the TreeBuilder itself, not in a file that we need to bundle. Commands should be responsible for finding the right configuration file to use.

@jaugustin
Propel member

One configuration to rule them all :) that's great

@cristianoc72
Propel member

@staabm before starting this work I spent some time in looking for "something" ready-to-reuse, in particular I got a look at Symfony internals. I've found only Dependency Injection Component and my work is heavily inspired on it. Unfortunately, the most part of the logic of those classes is about putting properties and services in the DI container, so it's impossible to reuse them as they are. I also made an experiment to try to reuse Symfony\Component\DependencyInjection\ParameterBag but it needed too many workarounds to be Propel compatible and, finally, I copy-pasted a couple of methods (even if I hate to do so).

@willdurand Good suggestion about defaults inside TreeBuilder: I'll do so.

@jaugustin jaugustin and 2 others commented on an outdated diff Jan 18, 2014
tools/generator/default-config.yaml
+schema:
+ validate: true
+ transform: false
+ autoPackage: false
+ autoNamespace: false
+ autoPrefix: false
+
+# -------------------------------------------------------------------
+#
+# D A T A B A S E S E T T I N G S
+#
+# -------------------------------------------------------------------
+database:
+ runtime:
+ datasources:
+ #riportare qui il runtime conf
@cristianoc72
cristianoc72 Jan 19, 2014

Oh sorry mates! It means 'move here runtime.conf content'. Anyway, I'm going to remove this file, to put defaults into the TreeBuilder.

@gossi

Whoa! So great and one config for runtime and generator. This is what the world has been waited for 😄 Thank you so much.

I have two wishes:

  1. Please use propel.ext as config file name (instead of propel-config.ext) because mostly projects in their root (or config folders) contain of config files for tools. (eg phpunit) and I think propel should continue with that trend

  2. Also look for a .dist file. So, if a project uses propel, they should use propel.ext.dist as their config file, that also got committed into cvs. If you define connections to your local database they should be managed in propel.ext.

Thanks

@robin850
Propel member

👍 Awesome work @cristianoc72!

I'm wondering whether this would be possible to add an extra layer between AbstractCommand and few commands to make them have a --config-file option or something like this to specify the path to the configuration file ? Actually, something like database.yml could fit for tiny/medium projects but I guess you want to be sure that Propel doesn't clash with any other libraries and database.yml seems a bit generic.

If it needs too much work/code then I don't think this is really needed but this is just a proposal.

@cristianoc72
Propel member

@gossi @robin850 your suggestions have been implemented. Thanks!

@cristianoc72
Propel member

Sample configuration file (in yaml format, by now)
https://gist.github.com/cristianoc72/9060420

@willdurand
Propel member

❤️

@gossi

Cool.

I did some restructuring (grouping topic based settings), removed redundancy, slicked it down. Removed 'propel.behaviors' (obsolete since #511) See it here: https://gist.github.com/gossi/9072077. I marked some settings I think should be removed as deprecated. This is how I as a propel user would like to write my config.

@cristianoc72
Propel member

@gossi Good! I'll take your structure. I'll change only the connection parameter to accept more then one connections:

runtime:
   connections:
        - Mickey
        - Donald 
        - ...........

    defaultConnection: Donald 

About deprecated properties: I've considered only those which are mentioned in the code one time at least. Anyway, I'm sure that we'll find some other deprecated or not more useful properties while refactoring.

@cristianoc72
Propel member

Current status: all classes are refactored, and specific tests are green.
Now, I'm running test suite to fix "error-by-error".
What do you thing about removing a couple of deprecated properties? tablePrefix and targetPackage

@marcj
Propel member

👯 👍

@staabm
Propel member

it would be better to remove the deprecated properties in a separate PR to ease review?

@cristianoc72
Propel member

@staabm usually yes. In this case, I'm asking you because there are only 5 -6 lines of code to remove.

@willdurand
Propel member

What the status of this?

@willdurand willdurand added this to the 2.0.0 milestone Mar 16, 2014
@cristianoc72
Propel member

I'm fixing the test suite. I think I do it in a couple of weeks. It's a huge work: whenever I fix an error another one pops out!

@marcj
Propel member

Can I help somewhere?

@cristianoc72
Propel member

Sure! I've added you as a collaborator to my Propel2 repository. All the code is in config-refactor branch. I'm going to push everything up to now. Run test suite and enjoy 😄

@cristianoc72
Propel member

Done! While you're reviewing I'm goning to write the documentation.

@cristianoc72
Propel member

Should I squash those three commits into only one? In the third commit there are some fixes for the previous two...

@staabm
Propel member

After we finished the review it should be squashed.. Atm it is easier to handle as separate commits before we land it.

@willdurand
Propel member

well, no need to squash these commits IMO. Commit messages explain what is shipped.

@marcj
Propel member

(keep in mind #587)

@marcj
Propel member

How about adding support for env variables?

@cristianoc72
Propel member
@marcj
Propel member

I actually mean some placeholder for environment variables. Many hoster provide their credentials and database names etc via environment variables, so something like that would be cool, I guess:

# yaml
database:
    adapter: mysql
    host: localhost
    dsn: %env.adapter%:host=%env.host%
@cristianoc72
Propel member

Sure! I can refactor Propel\Common\Config\Loader\FileLoader::parseString method, to replace %env.variable_name% with the value of $_ENV['variable_name']. It isn't difficult.

@cristianoc72
Propel member

I'm confirming that the new configuration system isn't affected of #587.
I run the command with the following schema:

<?xml version="1.0" encoding="ISO-8859-1" standalone="no"?>
<database name="default" defaultIdMethod="native" >

    <table name="book" description="Book Table">
        <column name="id" required="true" primaryKey="true" autoIncrement="true" type="INTEGER" description="Book Id" />
        <column name="title" type="VARCHAR" required="true" description="Book Title" primaryString="true" />
        <column name="isbn" required="true" type="VARCHAR" size="24" phpName="ISBN" description="ISBN Number" primaryString="false" />
        <column name="price" required="false" type="FLOAT" description="Price of the book." />
        <column name="publisher_id" required="false" type="INTEGER" description="Foreign Key Publisher" />
        <column name="author_id" required="false" type="INTEGER" description="Foreign Key Author" />
        <foreign-key foreignTable="publisher" onDelete="setnull">
            <reference local="publisher_id" foreign="id" />
        </foreign-key>
        <foreign-key foreignTable="author" onDelete="setnull" onUpdate="cascade">
            <reference local="author_id" foreign="id" />
        </foreign-key>
    </table>

    <table name="publisher" description="Publisher Table" defaultStringFormat="XML">
        <column name="id" required="true" primaryKey="true" autoIncrement="true" type="INTEGER" description="Publisher Id" />
        <column name="name" required="true" type="VARCHAR" size="128" default="Penguin" description="Publisher Name" />
    </table>

    <table name="author" description="Author Table">
        <column name="id" required="true" primaryKey="true" autoIncrement="true" type="INTEGER" description="Author Id" />
        <column name="first_name" required="true" type="VARCHAR" size="128" description="First Name" />
        <column name="last_name" required="true" type="VARCHAR" size="128" description="Last Name" />
        <column name="email" type="VARCHAR" size="128" description="E-Mail Address" />
        <column name="age" type="INTEGER" description="The authors age" />
    </table>

</database>

and the following configuration:

propel:
  database:
      connections:
          default:
              adapter: mysql
              classname: Propel\Runtime\Connection\DebugPDO
              dsn: mysql:host=localhost;dbname=test
              user: root
              password: 
  runtime:
      defaultConnection: default
      connections:
          - default
  generator:
      defaultConnection: default
      connections:
          - default

and the output is what we expect:

<?php
$serviceContainer = \Propel\Runtime\Propel::getServiceContainer();
$serviceContainer->checkVersion('2.0.0-dev');
$serviceContainer->setAdapterClass('default', 'mysql');
$manager = new \Propel\Runtime\Connection\ConnectionManagerSingle();
$manager->setConfiguration(array (
  'classname' => 'Propel\\Runtime\\Connection\\DebugPDO',
  'dsn' => 'mysql:host=localhost;dbname=test',
  'user' => 'root',
  'password' => '',
));
$manager->setName('default');
$serviceContainer->setConnectionManager('default', $manager);
$serviceContainer->setDefaultDatasource('default');
@cristianoc72
Propel member

Added support for environment variables (see Propel\Tests\Common\Config\FileLoaderTest) and rebased.

@marcj
Propel member

Cool! Can't wait to see that baby in action :-)

@marcj
Propel member

@cristianoc72, does this refactoring also resolve #453?

@cristianoc72
Propel member

Yep! You can configure every connections in propel-datadase-connections section and choose one or more for build-time in propel-generator-connections section, which replaces buildtime-conf.xml.
propel-runtime-connections plus propel-runtime-log plus propel-runtime-profiler sections replace runtime-conf.xml.

By now, see https://gist.github.com/cristianoc72/9060420

I'm writing the reference documentation in these nights, I hope to give you an Easter gift 😄

@robin850
Propel member

@cristianoc72 : ❤️ 🍰 👍

@marcj
Propel member

Very very very cool! ❤️ 🍸 👍

@cristianoc72
Propel member

Sorry mates, maybe I've lost something: why Travis doesn't run test:prepare command anymore? How can I emulate Travis behavior locally?
If I run test:prepare command all tests are green but when Travis builds there are a lot of failures, because there aren't fixtures and configuration files.
Please, could you explain me? 😕

@marcj
Propel member

Yeah, there's no need to run test:prepare manually anymore. I'll explain it in detail in propelorm/propelorm.github.com#286. in short:

DB=mysql phpunit

Fixtures will automatically be built. If you change the DB env var, it will be automatically detected and will trigger a rebuild.

@cristianoc72
Propel member

Cool!! I'm going to fix my pr. Thanks!

cristianoc72 added some commits Jan 16, 2014
@cristianoc72 cristianoc72 Configuration system refactor - step 1
Configuration system refactor, according to issue 285:
- Add support for various configuration formats
- Create a more rational configuration hierarchy
- Validate the configuration values

Step 1: add new classes and relative tests
689fc51
@cristianoc72 cristianoc72 Configuration system refactor - step 2
Implement Propel\Common\Config\PropelConfiguration class to validate configuration
and load defaults, via Symfony\Component\Config\Definition\Builder\TreeBuilder class.
Remove all not more used options and define a better property structure.
Add the possibility to define some properties at runtime (i.e. passing some values
from command line)
0d7d507
@cristianoc72 cristianoc72 Configuration System Refactor - Step 3
Refactor all classes, tests and fixtures which use configurations.
Add support for environment variables.
ad3eb32
@cristianoc72 cristianoc72 referenced this pull request in propelorm/propelorm.github.com May 19, 2014
Merged

Configuration system refactor: documentation #296

@cristianoc72
Propel member

PR for documentation update just submitted. See propelorm/propelorm.github.com#296

Now, I'm going to fix hhvm tests.

@cristianoc72
Propel member

Hhvm failures are related to a Symfony Finder issue. When you specify depth(0) no file nor directory is found. With Zend Engine everything works as expected.
I'll submit an issue, with failed test case, on Symfony repo as soon as possible.

@marcj
Propel member

Yeah, hhvm is not that important right now.

@marcj
Propel member

Is hhvm the only issue here?

@cristianoc72
Propel member
@marcj marcj merged commit 753bcc9 into propelorm:master Jul 15, 2014

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
@marcj
Propel member

Very cool, thanks for your really awesome job Cristiano! 👍 👍 🎉

@cristianoc72
Propel member

Thank you mates, for your support and your suggestions! Long live to Propel!

@marcj
Propel member

Hehe Yeah, you deserved some beers from me. 😄 ❤️ Cristiano

@staabm
Propel member

👏

@willdurand
Propel member

yay! Congratulations! ❤️ 🎆 🔥 🎉

@gossi

Awesome, thank you very much 🍻

@cristianoc72 cristianoc72 deleted the cristianoc72:config-refactor branch Jul 22, 2014
@cristianoc72 cristianoc72 added a commit to cristianoc72/propelorm.github.com that referenced this pull request Jul 26, 2014
@cristianoc72 cristianoc72 Configuration system refactor: documentation
Update the documentation while refactoring Propel2 configuration system.
See propelorm/Propel2#527
2323790
@cristianoc72 cristianoc72 added a commit to cristianoc72/propelorm.github.com that referenced this pull request Aug 2, 2014
@cristianoc72 cristianoc72 Configuration system refactor: documentation
Update the documentation while refactoring Propel2 configuration system.
See propelorm/Propel2#527
8457403
@robin850 robin850 referenced this pull request in propelorm/propelorm.github.com Aug 29, 2014
Closed

Page Not Found: Customizing Build #316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment