Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Filesystem] Consistence and enhancements for Filesystem#4330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
travisbot commentedMay 18, 2012
fabpot commentedMay 20, 2012
To be consistent, we should throw exception whenever some operation fails. |
romainneutron commentedMay 20, 2012
I fix the consistency ; mkdir now throws an exception if a directory creation fails. Added chgrp and chown methods |
travisbot commentedMay 20, 2012
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You need to use:null === $time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed !
travisbot commentedMay 22, 2012
travisbot commentedMay 22, 2012
travisbot commentedMay 22, 2012
travisbot commentedMay 23, 2012
travisbot commentedMay 23, 2012
travisbot commentedMay 29, 2012
travisbot commentedMay 29, 2012
romainneutron commentedJun 1, 2012
Any news about this PR ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you should remove the line :
* @return Boolean true if the directory has been created, false otherwiseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
done, and exception doc added
stloyd commentedJun 8, 2012
@romainneutron You need tosquash your commits, and add more proper message in squashed commit i.e.:
|
romainneutron commentedJun 8, 2012
@stloyd squash done ! |
travisbot commentedJun 8, 2012
travisbot commentedJun 8, 2012
romainneutron commentedJun 9, 2012
I've added documentation to the Filesystem component :symfony/symfony-docs#1439 |
travisbot commentedJun 9, 2012
stloyd commentedJun 13, 2012
@romainneutron You probably need to rebase your code as some changes were merge into master for |
romainneutron commentedJun 13, 2012
@stloyd rebase OK ! by the way, do you have any idea when/if this PR will be merged ? |
travisbot commentedJun 13, 2012
fabpot commentedJun 16, 2012
You need to add a note about the BC breaks in the CHANGELOG file. |
fabpot commentedJun 16, 2012
Also, instead of using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shouldn't an "@" be added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
travisbot commentedJun 18, 2012
stloyd commentedJun 18, 2012
@fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR) |
romainneutron commentedJun 18, 2012
romainneutron commentedJun 18, 2012
@fabpot I've added the exception and the exception interface, add the changelog info |
travisbot commentedJun 18, 2012
romainneutron commentedJun 18, 2012
As reported by@stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved |
travisbot commentedJun 18, 2012
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is wrong, you need to adduse Symfony\Component\Filesystem\Exception\IOException; and use only class name here and all way below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this has been fixed
travisbot commentedJun 18, 2012
travisbot commentedJun 18, 2012
[FileSystem] explain possible failure of symlink creation in windows
travisbot commentedJun 19, 2012
Commits-------a20fc68 Merge pull request#1 from SamsonIT/FilesystemExceptions8eca661 [FileSystem] explains possible failure of symlink creation in windowsb1f8744 Add Changelog BC Break note24eb396 [Filesystem] Added few new behaviors:Discussion----------[Filesystem] Consistence and enhancements for FilesystemBug fix: noFeature addition: yesBackwards compatibility break: **yes**Symfony2 tests pass: yesFixes the following tickets: NoneLicense of the code: MITThis PR adds features and introduce a backward compatibility break.features :- whenever an action fails, a \RuntimeException is thrown- add access to the second and third arguments of ``touch`` function- add a recursive option for chmod- add a chown method- add a chgrp methodThe backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise.It now returns nothing.---------------------------------------------------------------------------by travisbot at 2012-05-18T14:26:42ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into1e15f21).---------------------------------------------------------------------------by fabpot at 2012-05-20T02:40:28ZTo be consistent, we should throw exception whenever some operation fails.---------------------------------------------------------------------------by romainneutron at 2012-05-20T21:10:23ZI fix the consistency ; mkdir now throws an exception if a directory creation fails.This introduce a BC break, see PR message which has been updated with all features and BC break.Added chgrp and chown methodsAdd options for touchAdd recursive option for chmod---------------------------------------------------------------------------by travisbot at 2012-05-20T21:11:47ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into1407f11).---------------------------------------------------------------------------by travisbot at 2012-05-22T10:49:06ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-22T10:58:10ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-22T11:18:44ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-23T18:21:47ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-23T18:26:19ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into517ae43).---------------------------------------------------------------------------by travisbot at 2012-05-29T16:07:26ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e intoadf07f1).---------------------------------------------------------------------------by travisbot at 2012-05-29T16:19:38ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa intoadf07f1).---------------------------------------------------------------------------by romainneutron at 2012-06-01T14:30:45ZAny news about this PR ?---------------------------------------------------------------------------by stloyd at 2012-06-08T09:57:39Z@romainneutron You need to [squash](http://www.silverwareconsulting.com/index.cfm/2010/6/6/Using-Git-Rebase-to-Squash-Commits) your commits, and add more proper message in squashed commit i.e.:> [Filesystem] Added few new behaviors:* whenever an action fails, a `RuntimeException` is thrown* add access to the second and third arguments of `touch()` function* add a recursive option for `chmod()`* add a `chown()` method* add a `chgrp()` method> BC break: `mkdir()` function throw exception in case of failture instead of returning Boolean value.---------------------------------------------------------------------------by romainneutron at 2012-06-08T10:59:55Z@stloyd squash done !---------------------------------------------------------------------------by travisbot at 2012-06-08T11:26:20ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 intof8e68e5).---------------------------------------------------------------------------by travisbot at 2012-06-08T11:41:45ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 intof8e68e5).---------------------------------------------------------------------------by romainneutron at 2012-06-09T11:42:24ZI've added documentation to the Filesystem component :symfony/symfony-docs#1439---------------------------------------------------------------------------by travisbot at 2012-06-09T16:47:20ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 intof8a09db).---------------------------------------------------------------------------by stloyd at 2012-06-13T14:47:31Z@romainneutron You probably need to rebase your code as some changes were merge into master for `Filesystem`.---------------------------------------------------------------------------by romainneutron at 2012-06-13T15:17:31Z@stloyd rebase OK !by the way, do you have any idea when/if this PR will be merged ?---------------------------------------------------------------------------by travisbot at 2012-06-13T15:20:44ZThis pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 intoc07e916).---------------------------------------------------------------------------by fabpot at 2012-06-16T16:40:50ZYou need to add a note about the BC breaks in the CHANGELOG file.---------------------------------------------------------------------------by fabpot at 2012-06-16T16:43:20ZAlso, instead of using `\RuntimeException`, I would create a custom exception like we have done in other components (an interface + a RuntimeException that implements the interface and extends \RuntimeException). The exception name can be something like `IOException`.---------------------------------------------------------------------------by travisbot at 2012-06-18T10:11:20ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into0b8b76b).---------------------------------------------------------------------------by stloyd at 2012-06-18T10:14:52Z@fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR)---------------------------------------------------------------------------by romainneutron at 2012-06-18T10:29:20Z@fabpot@stloyd the latest push was just a rebase push for PR 4577 (#4577)I'm currently fixing the Exception and changelog things, I'll push very soon---------------------------------------------------------------------------by romainneutron at 2012-06-18T10:44:38Z@fabpot I've added the exception and the exception interface, add the changelog info---------------------------------------------------------------------------by travisbot at 2012-06-18T10:53:34ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into0b8b76b).---------------------------------------------------------------------------by romainneutron at 2012-06-18T11:08:43ZAs reported by@stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved---------------------------------------------------------------------------by travisbot at 2012-06-18T11:16:58ZThis pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into0b8b76b).
willdurand commentedJun 19, 2012
0 additions, 0 deletions? :o |
romainneutron commentedJun 19, 2012
@willdurand it seems to be a github bug here are the commits |
Bug fix: no
Feature addition: yes
Backwards compatibility break:yes
Symfony2 tests pass: yes
Fixes the following tickets: None
License of the code: MIT
This PR adds features and introduce a backward compatibility break.
features :
touchfunctionThe backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise.
It now returns nothing.