About FileSystem (probably not for 1.3)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

About FileSystem (probably not for 1.3)

Stéphane Ducasse
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
Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Igor Stasenko
On 26 January 2011 21:45, Stéphane Ducasse <[hidden email]> 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.

Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Max Leske
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 <[hidden email]> 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.


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Tudor Girba
In reply to this post by Igor Stasenko
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 <[hidden email]> 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."




Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Max Leske

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 <[hidden email]> 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.
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Tudor Girba

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


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Alexandre Bergel
> ****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
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.






Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Max Leske
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
>

Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Dale Henrichs
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
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
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
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Dale Henrichs
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
>>>>
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Lukas Renggli
In reply to this post by Stéphane Ducasse
> 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

Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Dale Henrichs
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

Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Dale Henrichs

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
>>>>>
>>>>
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Lukas Renggli

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
>


Reply | Threaded
Open this post in threaded view
|

Re: About FileSystem (probably not for 1.3)

Stéphane Ducasse
In reply to this post by Dale Henrichs
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
>