[Pharo-project] About FileSystem (probably not for 1.3)
Hi guys, I was checking the state of FileSystem and we cannot push it into pharo like that! Sad but we should not put code where not a single method has a comment. So instead of crying and bashing colin, I started a while ago to understand (great pain) and write comments probably wrong, of course. Now I did several iterations but not enough. So I will read the blog entries of lukas and try to continue. http://www.lukas-renggli.ch/blog/filesystem Now if you want pharo to evolve, then this is clearly a place to help. We should get better class comments, and all the methods with a comment. Else FileSystem will not get integrated. I will veto it. Colin lost all the changes he did after ESUG so we will have to live with that (for example FSPath not being a class for public consumption and others). Stef
On 26 January 2011 21:45, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Hi guys,
I was checking the state of FileSystem and we cannot push it into pharo like that! Sad but we should not put code where not a single method has a comment. So instead of crying and bashing colin, I started a while ago to understand (great pain) and write comments probably wrong, of course. Now I did several iterations but not enough. So I will read the blog entries of lukas and try to continue.
http://www.lukas-renggli.ch/blog/filesystem
Now if you want pharo to evolve, then this is clearly a place to help. We should get better class comments, and all the methods with a comment. Else FileSystem will not get integrated. I will veto it.
Is it incomplete , or it just not well documented? I understand that new things require documentation because people won't know how to use it, but what about functionality?
Colin lost all the changes he did after ESUG so we will have to live with that (for example FSPath not being a class for public consumption and others).
Stef
-- Best regards, Igor Stasenko AKA sig.
I had to make some changes / additions (only very few though) to Filesystem myself. You can find the changes here: http://www.squeaksource.com/GitFS. A few of the changes have been integrated into the main branch by Lukas I think. I'd be happy to help with Filesystem especially since I've worked with it quite a bit. Unfortunately, I won't have time for another three or four weeks. Anyway, I'll keep following this thread. I'd really like to see a refined version of Filesystem pushed into Pharo. Max On 27.01.2011, at 09:59, Igor Stasenko wrote:
On 26 January 2011 21:45, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Hi guys,
I was checking the state of FileSystem and we cannot push it into pharo like that! Sad but we should not put code where not a single method has a comment. So instead of crying and bashing colin, I started a while ago to understand (great pain) and write comments probably wrong, of course. Now I did several iterations but not enough. So I will read the blog entries of lukas and try to continue.
http://www.lukas-renggli.ch/blog/filesystem
Now if you want pharo to evolve, then this is clearly a place to help. We should get better class comments, and all the methods with a comment. Else FileSystem will not get integrated. I will veto it.
Is it incomplete , or it just not well documented? I understand that new things require documentation because people won't know how to use it, but what about functionality?
Colin lost all the changes he did after ESUG so we will have to live with that (for example FSPath not being a class for public consumption and others).
Stef
-- Best regards, Igor Stasenko AKA sig.
On Jan 27, 2011, at 10:06 AM, Max Leske wrote:
I had to make some changes / additions (only very few though) to Filesystem myself. You can find the changes here: http://www.squeaksource.com/GitFS. A few of the changes have been integrated into the main branch by Lukas I think. I'd be happy to help with Filesystem especially since I've worked with it quite a bit. Unfortunately, I won't have time for another three or four weeks. Anyway, I'll keep following this thread. I'd really like to see a refined version of Filesystem pushed into Pharo.
Thanks I will merge your changes first. For example I do not like filesystem workingDirectory isWorkingDirectory returns false so workingDirectory should be renamed workingPath and FS... workingDirectory -> should be as current workingDirectory I started to add comments as far as I could understand the library. I will read the tests and probably will write new ones to document what I understand.
Max
On 27.01.2011, at 09:59, Igor Stasenko wrote:
On 26 January 2011 21:45, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Hi guys,
I was checking the state of FileSystem and we cannot push it into pharo like that! Sad but we should not put code where not a single method has a comment. So instead of crying and bashing colin, I started a while ago to understand (great pain) and write comments probably wrong, of course. Now I did several iterations but not enough. So I will read the blog entries of lukas and try to continue.
http://www.lukas-renggli.ch/blog/filesystem
Now if you want pharo to evolve, then this is clearly a place to help. We should get better class comments, and all the methods with a comment. Else FileSystem will not get integrated. I will veto it.
Is it incomplete , or it just not well documented? I understand that new things require documentation because people won't know how to use it, but what about functionality?
Colin lost all the changes he did after ESUG so we will have to live with that (for example FSPath not being a class for public consumption and others).
Stef
-- Best regards, Igor Stasenko AKA sig.
Hi max I started to merge some of your changes to FS. Now I was wondering about this one: ***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists Could you explain us a bit more? Stef
Stef, I have ported FileSystem to GemStone, but I had to make a number of changes that were necessary to improve the portability (at least for GemStone) ... It would be a bit of work for me to extract the changes, but I'd be willing to do it if you were interested in incorporated some of them (if not all) ... Dale On 01/27/2011 12:58 PM, Stéphane Ducasse wrote:
Hi max
I started to merge some of your changes to FS. Now I was wondering about this one:
***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
Stef
I'm. :) I do not want differences for the shake for them. I want an excellent system. I'm working on PharoTaskForces so if you can merge step by step it would be great. Stef On Jan 27, 2011, at 10:31 PM, Dale Henrichs wrote:
Stef,
I have ported FileSystem to GemStone, but I had to make a number of changes that were necessary to improve the portability (at least for GemStone) ... It would be a bit of work for me to extract the changes, but I'd be willing to do it if you were interested in incorporated some of them (if not all) ...
Dale
On 01/27/2011 12:58 PM, Stéphane Ducasse wrote:
Hi max
I started to merge some of your changes to FS. Now I was wondering about this one:
***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
Stef
Sounds good ... I'll work through the changes that I've made and try to extract out the goodness and then provide suggested changes (in the form of an mcz file you can merge if the changes make sense to you)... BTW, I used the FileSystem port with a Monticello port (replacing all of the FileDirectory stuff with FileSystem calls) and the Monticello port was based on a fairly recent Pharo version of Monticello:) Dale On 01/27/2011 01:34 PM, Stéphane Ducasse wrote:
I'm. :) I do not want differences for the shake for them. I want an excellent system. I'm working on PharoTaskForces so if you can merge step by step it would be great.
Stef
On Jan 27, 2011, at 10:31 PM, Dale Henrichs wrote:
Stef,
I have ported FileSystem to GemStone, but I had to make a number of changes that were necessary to improve the portability (at least for GemStone) ... It would be a bit of work for me to extract the changes, but I'd be willing to do it if you were interested in incorporated some of them (if not all) ...
Dale
On 01/27/2011 12:58 PM, Stéphane Ducasse wrote:
Hi max
I started to merge some of your changes to FS. Now I was wondering about this one:
***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
Stef
On Jan 27, 2011, at 10:56 PM, Dale Henrichs wrote:
Sounds good ... I'll work through the changes that I've made and try to extract out the goodness and then provide suggested changes (in the form of an mcz file you can merge if the changes make sense to you)...
BTW, I used the FileSystem port with a Monticello port (replacing all of the FileDirectory stuff with FileSystem calls) and the Monticello port was based on a fairly recent Pharo version of Monticello:)
Excellent
Dale
On 01/27/2011 01:34 PM, Stéphane Ducasse wrote:
I'm. :) I do not want differences for the shake for them. I want an excellent system. I'm working on PharoTaskForces so if you can merge step by step it would be great.
Stef
On Jan 27, 2011, at 10:31 PM, Dale Henrichs wrote:
Stef,
I have ported FileSystem to GemStone, but I had to make a number of changes that were necessary to improve the portability (at least for GemStone) ... It would be a bit of work for me to extract the changes, but I'd be willing to do it if you were interested in incorporated some of them (if not all) ...
Dale
On 01/27/2011 12:58 PM, Stéphane Ducasse wrote:
Hi max
I started to merge some of your changes to FS. Now I was wondering about this one:
***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
Stef
I started to merge some of your changes to FS.
I already merged most of the changes of Max into the main code.
Now I was wondering about this one: ***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
This is not something that should go into the main code. If I remember correctly this was a quick hack to avoid some problems in GitFS. Lukas -- Lukas Renggli www.lukas-renggli.ch
On 01/27/2011 03:00 PM, Lukas Renggli wrote:
I started to merge some of your changes to FS.
I already merged most of the changes of Max into the main code.
Now I was wondering about this one: ***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
This is not something that should go into the main code. If I remember correctly this was a quick hack to avoid some problems in GitFS.
Lukas
At the time I ported FileSystem to GemStone (4 months ago?), I started with the packages in Lukas' repository... Dale
I merged them also in PharoTaskForces On Jan 28, 2011, at 12:02 AM, Dale Henrichs wrote:
On 01/27/2011 03:00 PM, Lukas Renggli wrote:
I started to merge some of your changes to FS.
I already merged most of the changes of Max into the main code.
Now I was wondering about this one: ***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
This is not something that should go into the main code. If I remember correctly this was a quick hack to avoid some problems in GitFS.
Lukas
At the time I ported FileSystem to GemStone (4 months ago?), I started with the packages in Lukas' repository...
Dale
On Jan 28, 2011, at 12:00 AM, Lukas Renggli wrote:
I started to merge some of your changes to FS.
I already merged most of the changes of Max into the main code.
Now I was wondering about this one: ***for my work on GitFS I needed a resumable exception (alternatively an ifPresent block would have done the trick but that would have ment more meddling with Filesystem) to delete existing files when copying. The following changes are a result of introducing this exception to the copy methods. *added class FSFileExistsResumbale and its superclass FSFilesystemException to support the changes to the copy methods *#copy:to: signals FSFileExistsResumbale instead of FSFileExists *FSCopyVisitor>>copyDirectory uses #ensureDirectory instead of #createDirectory *needed to change FSFilesystemTest>>testCopyDestExists to expect FSFileExistsResumable *FSReference>>basicCopyTo: signals FSFileExistsResumable instead of FSFileExists
Could you explain us a bit more?
This is not something that should go into the main code. If I remember correctly this was a quick hack to avoid some problems in GitFS.
Thanks lukas this is what I thought too. I picked some of the other elements. And I'm continuing to add comments. I will take your blog as starting material for a chapter. Stef
Lukas
-- Lukas Renggli www.lukas-renggli.ch
Hi Stef, I think your mail can easily be misunderstood to say that Filesystem is crap and that the authors did a bad job. I believe your intention was simply to push people to write more comments and give Filesystem as an example of a fine system that does not have comments :). Filesystem is a nice piece of work and the authors did a good job. I understand that there are no comments, but the documentation found on Lukas' website provides a better overview than most class comments in Pharo: http://www.lukas-renggli.ch/blog/filesystem-1 http://www.lukas-renggli.ch/blog/filesystem-2 So, why not just copy these into the help system? Cheers, Doru On 27 Jan 2011, at 09:59, Igor Stasenko wrote:
On 26 January 2011 21:45, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Hi guys,
I was checking the state of FileSystem and we cannot push it into pharo like that! Sad but we should not put code where not a single method has a comment. So instead of crying and bashing colin, I started a while ago to understand (great pain) and write comments probably wrong, of course. Now I did several iterations but not enough. So I will read the blog entries of lukas and try to continue.
http://www.lukas-renggli.ch/blog/filesystem
Now if you want pharo to evolve, then this is clearly a place to help. We should get better class comments, and all the methods with a comment. Else FileSystem will not get integrated. I will veto it.
Is it incomplete , or it just not well documented? I understand that new things require documentation because people won't know how to use it, but what about functionality?
Colin lost all the changes he did after ESUG so we will have to live with that (for example FSPath not being a class for public consumption and others).
Stef
-- Best regards, Igor Stasenko AKA sig.
-- www.tudorgirba.com "Yesterday is a fact. Tomorrow is a possibility. Today is a challenge."
On Jan 27, 2011, at 10:07 AM, Tudor Girba wrote:
Hi Stef,
I think your mail can easily be misunderstood to say that Filesystem is crap and that the authors did a bad job. I believe your intention was simply to push people to write more comments and give Filesystem as an example of a fine system that does not have comments :).
Exact :)
Filesystem is a nice piece of work and the authors did a good job.
yes
I understand that there are no comments, but the documentation found on Lukas' website provides a better overview than most class comments in Pharo: http://www.lukas-renggli.ch/blog/filesystem-1 http://www.lukas-renggli.ch/blog/filesystem-2
So, why not just copy these into the help system?
This is not a solution. This would be a workaround. This is incomplete and missing important information. Then we should also review some api like workingDirectory returning a path. I started to write important class comments like DO NOT USE subclasses of FSDisk.... directly. Let FSDiskFilesystem returns the current filesystem. FSPath is for internal framework use. That I learned the hard way = spending 5 times more time than expected to do something. Documentation is not a by product. Finally I started to reorganize the package because there is no point directing people to **I want a comment for every single method.** I hate a system that tells me in permanence that I'm an idiot because what is written is not obvious to me. Sorry to be stubborn but I will not move on micrometer on that. ****Code without documentation and comments does not exist*** If the community wants a better filesystem then we should have comments in FS. Since I want it I will slowly and painfully try to understand that I'm idiot because I do not understand and reverse engineer what could have been written up front. I will slowly do it since there is no other paths. But I guess that you understand my deep state of mind. Stef
****Code without documentation and comments does not exist***
And tests. "A legacy application is an application that has no test" Alexandre -- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
participants (7)
-
Alexandre Bergel -
Dale Henrichs -
Igor Stasenko -
Lukas Renggli -
Max Leske -
Stéphane Ducasse -
Tudor Girba