You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/15 12:23:54 UTC
[GitHub] [commons-vfs] boris-petrov opened a new pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
boris-petrov opened a new pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150
I think this is a good default. Most providers won't do anything different than this. I guess? @garydgregory thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] boris-petrov commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
boris-petrov commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-746511940
@garydgregory - I spent a lot of time trying to figure what's going on. I give up. This test failure is definitely a bug in the `RamFileProvider` but it is difficult to fix. I changed a bunch of things and always something fails.
If you have the time, I would suggest you look into this and try fixing it.
Things that I saw which are not OK:
1) `AbstractFileObject::injectType` is only overridden in `RamFileObject` which seems fishy. I would try to remove this method.
1) `AbstractFileObject::endOutput` is a strange method. It is overridden only in `RamFileObject` which, again, is not nice.
Check also #151. I fixed some more "bad" things in the code around this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] boris-petrov commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
boris-petrov commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-752227935
@garydgregory - I rebased on master.
Please see [this comment](https://github.com/apache/commons-vfs/pull/150#issuecomment-746511940). As I said there, the failing test is a real issue in the `RAM` provider.
Also, please merge #151 before this. It simplifies some code and will help with debugging this issue here.
As for tests, this is not fixing any issues, it's just a convenience for some file systems that won't have to do anything "manually" `onChange`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory edited a comment on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-752099832
Hi @boris-petrov
As you may have seen in the check list, this PR breaks the build:
```
Tests run: 93, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 5.007 sec <<< FAILURE! - in org.apache.commons.vfs2.provider.ram.test.RamProviderTestCase
testListener(org.apache.commons.vfs2.test.ProviderWriteTests) Time elapsed: 0.007 sec <<< FAILURE!
junit.framework.AssertionFailedError: ram:///write-tests/newfile
```
Please rebase on master to use the latest but I am thinking that it might be your actual changes that brake the test.
Also this PR does that have a test or tests that fail without your change to the main side of the source tree.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] boris-petrov commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
boris-petrov commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-747258709
@garydgregory - done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-752099832
Hi @boris-petrov
As you may have seen in the check list, this PR breaks the build:
```
Tests run: 93, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 5.007 sec <<< FAILURE! - in org.apache.commons.vfs2.provider.ram.test.RamProviderTestCase
testListener(org.apache.commons.vfs2.test.ProviderWriteTests) Time elapsed: 0.007 sec <<< FAILURE!
junit.framework.AssertionFailedError: ram:///write-tests/newfile
```
Please rebase on master to use the latest but I am thinking that it might be your actual changes that brake the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-745262520
Hi @boris-petrov
Run a local build before you create a PR, as this would have show you this change breaks the build.
I am not sure of the semantics, needs studying... You PR would need a new test anyway to validate the change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] boris-petrov commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
boris-petrov commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-747480753
@garydgregory - done!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-747078789
Hi @boris-petrov
Please rebase on git master. TY!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-vfs] garydgregory commented on pull request #150: Call `refresh` by default in `AbstractFileObject.onChange`
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #150:
URL: https://github.com/apache/commons-vfs/pull/150#issuecomment-747478482
@boris-petrov
Please rebase on master to pickup better unit test assertion messages in `ProviderWriteTests`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org