[Filesystem] Consistence and enhancements for Filesystem#4330
Merged
fabpot merged 1 commit intosymfony:masterfrom Jun 19, 2012
Merged
[Filesystem] Consistence and enhancements for Filesystem#4330fabpot merged 1 commit intosymfony:masterfrom
fabpot merged 1 commit intosymfony:masterfrom
Conversation
Member
|
To be consistent, we should throw exception whenever some operation fails. |
Contributor
Author
|
I fix the consistency ; mkdir now throws an exception if a directory creation fails. Added chgrp and chown methods |
Contributor
There was a problem hiding this comment.
You need to use: null === $time.
Contributor
Author
|
Any news about this PR ? |
Contributor
There was a problem hiding this comment.
you should remove the line :
* @return Boolean true if the directory has been created, false otherwise
Contributor
Author
There was a problem hiding this comment.
done, and exception doc added
Contributor
|
@romainneutron You need to squash your commits, and add more proper message in squashed commit i.e.:
|
Contributor
Author
|
@stloyd squash done ! |
Contributor
Author
|
I've added documentation to the Filesystem component : symfony/symfony-docs#1439 |
Contributor
|
@romainneutron You probably need to rebase your code as some changes were merge into master for |
Contributor
Author
|
@stloyd rebase OK ! by the way, do you have any idea when/if this PR will be merged ? |
Member
|
You need to add a note about the BC breaks in the CHANGELOG file. |
Member
|
Also, instead of using |
Contributor
There was a problem hiding this comment.
Shouldn't an "@" be added here?
Contributor
|
@fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR) |
Contributor
Author
Contributor
Author
|
@fabpot I've added the exception and the exception interface, add the changelog info |
Contributor
Author
|
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 |
Contributor
There was a problem hiding this comment.
This is wrong, you need to add use Symfony\Component\Filesystem\Exception\IOException; and use only class name here and all way below.
Contributor
Author
There was a problem hiding this comment.
this has been fixed
[FileSystem] explain possible failure of symlink creation in windows
fabpot
added a commit
that referenced
this pull request
Jun 19, 2012
Commits ------- a20fc68 Merge pull request #1 from SamsonIT/FilesystemExceptions 8eca661 [FileSystem] explains possible failure of symlink creation in windows b1f8744 Add Changelog BC Break note 24eb396 [Filesystem] Added few new behaviors: Discussion ---------- [Filesystem] Consistence and enhancements for Filesystem 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 : - 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 The 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:42Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into 1e15f21). --------------------------------------------------------------------------- by fabpot at 2012-05-20T02:40:28Z To be consistent, we should throw exception whenever some operation fails. --------------------------------------------------------------------------- by romainneutron at 2012-05-20T21:10:23Z I 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 methods Add options for touch Add recursive option for chmod --------------------------------------------------------------------------- by travisbot at 2012-05-20T21:11:47Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into 1407f11). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:49:06Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:58:10Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T11:18:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:21:47Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:26:19Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:07:26Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e into adf07f1). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:19:38Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa into adf07f1). --------------------------------------------------------------------------- by romainneutron at 2012-06-01T14:30:45Z Any 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:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 into f8e68e5). --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:41:45Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 into f8e68e5). --------------------------------------------------------------------------- by romainneutron at 2012-06-09T11:42:24Z I've added documentation to the Filesystem component : symfony/symfony-docs#1439 --------------------------------------------------------------------------- by travisbot at 2012-06-09T16:47:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 into f8a09db). --------------------------------------------------------------------------- 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:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 into c07e916). --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:40:50Z You need to add a note about the BC breaks in the CHANGELOG file. --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:43:20Z Also, 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:20Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into 0b8b76b). --------------------------------------------------------------------------- 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:34Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into 0b8b76b). --------------------------------------------------------------------------- by romainneutron at 2012-06-18T11:08:43Z 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 --------------------------------------------------------------------------- by travisbot at 2012-06-18T11:16:58Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into 0b8b76b).
Contributor
|
0 additions, 0 deletions? :o |
Contributor
Author
|
@willdurand it seems to be a github bug here are the commits |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.