Skip to content
This repository

Zend\Config - plugin loading, ZF2-354, CS fixes #1479

Closed
wants to merge 40 commits into from

11 participants

Denis Portnov Maks Ben Scholzen Matthew Weier O'Phinney Yanick Rochon kevin mulama Walter Dal Mut jmarien Evan Coury Enrico Zimuel Nicholas Calugar
Denis Portnov
  • bit of refactoring to utilize plugin loading
  • ability to register reader for file extension (functionality was there but no public method existed)
  • ZF2-354
  • CS and docblock fixes
Denis Portnov denixport commented on the diff June 11, 2012
library/Zend/Config/Writer/AbstractWriter.php
@@ -62,7 +63,7 @@ function($error, $message = '', $file = '', $line = 0) use ($filename) {
62 63
                 ), $error);
63 64
             }, E_WARNING
64 65
         );
65  
-        file_put_contents($filename, $this->toString($config), $exclusiveLock);
  66
+        file_put_contents($filename, $this->toString($config), $flags);
1
Denis Portnov
denixport added a note June 11, 2012

fixes ZF2-354

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Processor/Filter.php
@@ -69,7 +69,8 @@ public function setFilter(ZendFilter $filter)
69 69
      * Process
70 70
      * 
71 71
      * @param  Config $config
72  
-     * @return Config 
  72
+     * @return Config
  73
+     * @throws \Zend\Config\Exception\InvalidArgumentException
2
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Why do you use the FQDN of the exception here, instead of just Exception\InvalidArgumentException? Same for all other occurences.

Denis Portnov
denixport added a note June 12, 2012

Looks like my IDE doesn't recognize parent namespace. Will fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Reader/Ini.php
((5 lines not shown))
79 80
      */
80 81
     public function fromFile($filename)
81 82
     {
82  
-        if (!file_exists($filename)) {
83  
-            throw new Exception\RuntimeException("The file $filename doesn't exists.");
  83
+        if (!is_readable($filename)) {
  84
+            throw new Exception\RuntimeException("File '{$filename}' doesn't exist or not readable");
2
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Better go with sprintf() directly here. Same for all other occurences.

Denis Portnov
denixport added a note June 12, 2012

Agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Reader/Yaml.php
((24 lines not shown))
53 55
      */
54  
-    public function __construct($yamlDecoder=null) {
  56
+    public function __construct($yamlDecoder = null)
  57
+    {
55 58
         if (!empty($yamlDecoder)) {
1
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Can you updated this do a null comparision? $yamlDecoder !== null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Reader/Yaml.php
@@ -113,8 +122,9 @@ public function fromFile($filename)
113 122
      * fromString(): defined by Reader interface.
114 123
      *
115 124
      * @see    ReaderInterface::fromString()
116  
-     * @param  string $string
  125
+     * @param string $string
2
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Why did you remove the correct alignment here?

Denis Portnov
denixport added a note June 12, 2012

sorry, will fix that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Reader/Yaml.php
((5 lines not shown))
137 148
     /**
138 149
      * Process the array for @include
139  
-     * 
140  
-     * @param  array $data
141  
-     * @return array 
  150
+     *
  151
+     * @param array $data
1
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Same here… and probably other places?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/ReaderLoader.php
((18 lines not shown))
  18
+ * @license   http://framework.zend.com/license/new-bsd     New BSD License
  19
+ */
  20
+
  21
+namespace Zend\Config;
  22
+
  23
+use Zend\Loader\PluginClassLoader;
  24
+
  25
+/**
  26
+ * Plugin Class Loader implementation for config readers.
  27
+ *
  28
+ * @category   Zend
  29
+ * @package    Zend_Serializer
  30
+ * @copyright  Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
  31
+ * @license    http://framework.zend.com/license/new-bsd     New BSD License
  32
+ */
  33
+class ReaderLoader extends PluginClassLoader
2
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Do we actually need our own Loader here? Why not just let the Broker register those plugins by default?

Denis Portnov
denixport added a note June 12, 2012

I was basically following the practice established around the framework. But I can change that back, not sure if it adds any performance hit though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Writer/Yaml.php
((9 lines not shown))
39 40
     /**
40 41
      * Constructor
41 42
      * 
42  
-     * @param callable $yamlDecoder 
  43
+     * @param callback|string $yamlDecoder
3
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

callable is actually the correct term, as it is the valid typehint as of PHP 5.4.

Denis Portnov
denixport added a note June 12, 2012

right, but minimal version is 5.3.3, also phpDocumentor has it as callback (http://www.phpdoc.org/docs/latest/for-users/types.html)

Matthew Weier O'Phinney Owner

We should go with callable for forward compat purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/Writer/Yaml.php
((5 lines not shown))
53 56
     /**
54 57
      * Get callback for decoding YAML
55 58
      *
56  
-     * @return callable
  59
+     * @return callback
1
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tests/Zend/Config/Reader/AbstractReaderTestCase.php
@@ -50,7 +50,7 @@
50 50
     public function testMissingFile()
51 51
     {
52 52
         $filename = $this->getTestAssetPath('no-file');
53  
-        $this->setExpectedException('Zend\Config\Exception\RuntimeException', "The file $filename doesn't exists.");
  53
+        $this->setExpectedException('Zend\Config\Exception\RuntimeException', "doesn't exist");
1
Ben Scholzen Collaborator
DASPRiD added a note June 12, 2012

Why limiting the check for two words here?

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

fixed all the above, except ReaderLoader class, needs more discussion

Ben Scholzen

I've seen no other place where we include the Exceptions directly, always the Exception namespace.

Actually similar code is already there in master https://github.com/zendframework/zf2/blob/master/library/Zend/Config/Processor/Constant.php#L24
But if you find it not ok, I can change it back

library/Zend/Config/ReaderBroker.php
((6 lines not shown))
  6
+ * @copyright Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
  7
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
  8
+ * @package   Zend_Config
  9
+ */
  10
+
  11
+namespace Zend\Config;
  12
+
  13
+use Zend\Loader\PluginBroker;
  14
+use Zend\Config\Reader\ReaderInterface;
  15
+
  16
+/**
  17
+ * Broker for serializer adapter instances
  18
+ *
  19
+ * @category   Zend
  20
+ * @package    Zend_Serializer
  21
+ * @copyright  Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
1
Maks Collaborator
Maks3w added a note June 13, 2012

@copyright and @license are not needed in class docblocks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Config/ReaderLoader.php
((5 lines not shown))
  5
+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
  6
+ * @copyright Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
  7
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
  8
+ * @package   Zend_Config
  9
+ */
  10
+
  11
+namespace Zend\Config;
  12
+
  13
+use Zend\Loader\PluginClassLoader;
  14
+
  15
+/**
  16
+ * Plugin Class Loader implementation for config readers.
  17
+ *
  18
+ * @category   Zend
  19
+ * @package    Zend_Config
  20
+ * @copyright  Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
1
Maks Collaborator
Maks3w added a note June 13, 2012

the same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Maks
Collaborator
Maks3w commented June 14, 2012

Thank you @denixport the reason for that is I will open a PR soon for update all file headers to the new format and then doing your change we can avoid merge conflicts

Maks
Collaborator
Maks3w commented June 14, 2012

Also I recommend you add a comment when you add a commit because there aren't notifications for that to the reviewer.

@DASPRiD ping

kevin mulama

I was just wondering, can this tutorial work in a linux shared hosting account? if not can be done to make it function in a linux shared hosting account.

I think you're in the wrong place. Go on IRC, freenode.net and join #zftalk.2. There are plenty of Google hits if you need more information.

and others added some commits June 27, 2012
Enrico Zimuel Merge pull request #1444 from wdalmut/master
Zend\Mvc Quick Start Italian Translation
ea3f18f
Matthew Weier O'Phinney Merge branch 'feature/validator-cleanup' of https://github.com/Maks3w… 161e608
Matthew Weier O'Phinney Merge branch 'feature/ipvfuture-validator' of https://github.com/Maks…
…3w/zf2 into feature/1590
60e08be
Matthew Weier O'Phinney [#1590] Removed var_dump d12342a
Matthew Weier O'Phinney Merge branch 'feature/mail-add-headers-containter-to-parts' of https:…
…//github.com/Maks3w/zf2 into feature/mail-headers
73ece1a
Matthew Weier O'Phinney [#1555] minor CS cleanup
- space after foreach
fbd39d0
Nicholas Calugar Fixing FilterChain for plugins via ServiceManager.
Also, the word filters need to accept arrays or else they can't be attached with attachByName.
0df5ff9
Matthew Weier O'Phinney Merge branch 'hotfix/uri-validation' of https://github.com/Maks3w/zf2
…into hotfix/uri-ipv6
1ce6f77
Evan Coury Merge pull request #1620 from jmarien/fixes/captcha
small typos fixed in Exception namespaces for form element view helpers
79edf2b
Denis Portnov extended Factory, plugin loading and CS fixes b060c30
Denis Portnov registerExtension(), tests, more CS fixes c3c8dbc
Denis Portnov dockblock fixes, fix yaml_emit in YAML writer d0720dc
Denis Portnov revert back to previous exceptions e8765b8
Denis Portnov fixed dockblocks and headers c85fd1e
Denis Portnov implement PluginManager d8fcf1b
Denis Portnov Merge remote-tracking branch 'origin/hotfix/config' into hotfix/config
Conflicts:
	library/Zend/Config/Factory.php
	tests/Zend/Config/FactoryTest.php
4fc171b
Denis Portnov remove Broker/Loader 30689b7
Denis Portnov denixport closed this June 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 40 unique commits by 8 authors.

Jun 02, 2012
Walter Dal Mut minor fix. remove en d56cd70
Walter Dal Mut Added Italian translation for Zend_Mvc-QuickStart.xml 22766aa
Jun 04, 2012
Walter Dal Mut Fix a lot of translations 4ae47ed
Jun 05, 2012
Walter Dal Mut Zend_Captcha resource ita 14adffb
Walter Dal Mut Merge branch 'master' of git://github.com/zendframework/zf2 89f97f2
Walter Dal Mut Added missing translation ff286a1
Jun 12, 2012
Denis Portnov extended Factory, plugin loading and CS fixes b7d49f5
Denis Portnov registerExtension(), tests, more CS fixes 79d0683
Denis Portnov dockblock fixes, fix yaml_emit in YAML writer a41ee6f
Denis Portnov revert back to previous exceptions 11092b5
Jun 14, 2012
Denis Portnov fixed dockblocks and headers f4d7354
Walter Dal Mut Merge branch 'master' of git://github.com/zendframework/zf2 1bcbe0d
Walter Dal Mut @ezimuel suggestions 68d6793
Jun 21, 2012
Maks [Uri] Add IPv6 to the list of valid types by default
Replaces the changes made in #1524 (4367cfa)

[Uri] Explain better the concept of the bitmask for part types
c587eac
Jun 22, 2012
Maks [Mail] Use Headers container rather than array in Zend\Mail\Storage\Part b674420
Maks [Mail] Use Header::fromString where possible a9ed9d9
Jun 26, 2012
Maks [Validator] Code clean-up
* Fix PHPDoc
* Fix CS (remove underscore)
ce0bb6c
Maks [Validator] Remove deprecated code 36bf6ed
Maks [Validator] Fix a bug in setOptions 08331e1
Maks [Validator] Add support for IPvFuture and literal format
Add support to IPvFuture as described in RFC 3986
Add support for IPs in literal format
e683347
Maks [Validator] Rework some test for use mock-ups instead of testassets c6c0caa
jmarien small typos fixed in Exception namespaces for form element view helpers 11579ea
Jun 27, 2012
Evan Coury Merge pull request #1638 from macnibblet/macnibblet/hotfix/formselect…
…-selected-values

FormSelect cannot use strict comparison of values
7656aa7
Enrico Zimuel Merge pull request #1444 from wdalmut/master
Zend\Mvc Quick Start Italian Translation
ea3f18f
Matthew Weier O'Phinney Merge branch 'feature/validator-cleanup' of https://github.com/Maks3w… 161e608
Matthew Weier O'Phinney Merge branch 'feature/ipvfuture-validator' of https://github.com/Maks…
…3w/zf2 into feature/1590
60e08be
Matthew Weier O'Phinney [#1590] Removed var_dump d12342a
Matthew Weier O'Phinney Merge branch 'feature/mail-add-headers-containter-to-parts' of https:…
…//github.com/Maks3w/zf2 into feature/mail-headers
73ece1a
Matthew Weier O'Phinney [#1555] minor CS cleanup
- space after foreach
fbd39d0
Nicholas Calugar Fixing FilterChain for plugins via ServiceManager.
Also, the word filters need to accept arrays or else they can't be attached with attachByName.
0df5ff9
Matthew Weier O'Phinney Merge branch 'hotfix/uri-validation' of https://github.com/Maks3w/zf2
…into hotfix/uri-ipv6
1ce6f77
Evan Coury Merge pull request #1620 from jmarien/fixes/captcha
small typos fixed in Exception namespaces for form element view helpers
79edf2b
Jun 28, 2012
Denis Portnov extended Factory, plugin loading and CS fixes b060c30
Denis Portnov registerExtension(), tests, more CS fixes c3c8dbc
Denis Portnov dockblock fixes, fix yaml_emit in YAML writer d0720dc
Denis Portnov revert back to previous exceptions e8765b8
Denis Portnov fixed dockblocks and headers c85fd1e
Denis Portnov implement PluginManager d8fcf1b
Denis Portnov Merge remote-tracking branch 'origin/hotfix/config' into hotfix/config
Conflicts:
	library/Zend/Config/Factory.php
	tests/Zend/Config/FactoryTest.php
4fc171b
Denis Portnov remove Broker/Loader 30689b7
Something went wrong with that request. Please try again.