About Fogbugz 20165 Support-segment-path-printing

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

About Fogbugz 20165 Support-segment-path-printing

Stephane Ducasse-3
Hi alistair

I'm starting to review code that is in the PR pipeline.
https://github.com/pharo-project/pharo/pull/126

I noticed that you wrote in the slice

Changes since last slice:

- Change Path>>fullName to just print the path
- Fix absolute path strings
- Bug fix Path class>>from:delimiter:
- Add unit tests

But I only see in the PR three methods.
Can you check because I have the impression that we are losing code somewhere?

BTW I could not see the contents of the AlistairGrant.1 package
I tried on several images 70 and 60 latest

Stef

Reply | Threaded
Open this post in threaded view
|

Re: About Fogbugz 20165 Support-segment-path-printing

alistairgrant
Hi Stef,

Good pickup.

I also can't load
SLICE-Issue-20165-Support-segment-path-printing-AlistairGrant.1.
Somehow it is dependent on FileSystem-Core-AlistairGrant.223, and I
have no idea what that is (I don't remember ever creating it, and I
don't have a copy on my machine anywhere, or in any of my backups).

Fortunately I do have a changeset of the patch.  I'll send it through
as an attachment after I've sent this email in case it causes the
response to be blocked.

Do you or Pavel have a preference on how to get the PR fixed?  I'm
happy to generate a new PR if you or Pavel can close the existing one
(#126).

Thanks,
Alistair


On 30 June 2017 at 16:09, Stephane Ducasse <[hidden email]> wrote:

> Hi alistair
>
> I'm starting to review code that is in the PR pipeline.
> https://github.com/pharo-project/pharo/pull/126
>
> I noticed that you wrote in the slice
>
> Changes since last slice:
>
> - Change Path>>fullName to just print the path
> - Fix absolute path strings
> - Bug fix Path class>>from:delimiter:
> - Add unit tests
>
> But I only see in the PR three methods.
> Can you check because I have the impression that we are losing code somewhere?
>
> BTW I could not see the contents of the AlistairGrant.1 package
> I tried on several images 70 and 60 latest
>
> Stef
>

Reply | Threaded
Open this post in threaded view
|

Re: About Fogbugz 20165 Support-segment-path-printing

alistairgrant
Attached is the changeset as discussed in my last email.

Just to be clear, this is just for reference, I'm happy to redo the PR
if it will save you and/or Pavel some time.

Cheers,
Alistair


On 30 June 2017 at 16:42, Alistair Grant <[hidden email]> wrote:

> Hi Stef,
>
> Good pickup.
>
> I also can't load
> SLICE-Issue-20165-Support-segment-path-printing-AlistairGrant.1.
> Somehow it is dependent on FileSystem-Core-AlistairGrant.223, and I
> have no idea what that is (I don't remember ever creating it, and I
> don't have a copy on my machine anywhere, or in any of my backups).
>
> Fortunately I do have a changeset of the patch.  I'll send it through
> as an attachment after I've sent this email in case it causes the
> response to be blocked.
>
> Do you or Pavel have a preference on how to get the PR fixed?  I'm
> happy to generate a new PR if you or Pavel can close the existing one
> (#126).
>
> Thanks,
> Alistair
>
>
> On 30 June 2017 at 16:09, Stephane Ducasse <[hidden email]> wrote:
>> Hi alistair
>>
>> I'm starting to review code that is in the PR pipeline.
>> https://github.com/pharo-project/pharo/pull/126
>>
>> I noticed that you wrote in the slice
>>
>> Changes since last slice:
>>
>> - Change Path>>fullName to just print the path
>> - Fix absolute path strings
>> - Bug fix Path class>>from:delimiter:
>> - Add unit tests
>>
>> But I only see in the PR three methods.
>> Can you check because I have the impression that we are losing code somewhere?
>>
>> BTW I could not see the contents of the AlistairGrant.1 package
>> I tried on several images 70 and 60 latest
>>
>> Stef
>>

Issue20165.cs (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: About Fogbugz 20165 Support-segment-path-printing

Pavel Krivanek-3
The PR is updated

-- Pavel

2017-06-30 16:44 GMT+02:00 Alistair Grant <[hidden email]>:
Attached is the changeset as discussed in my last email.

Just to be clear, this is just for reference, I'm happy to redo the PR
if it will save you and/or Pavel some time.

Cheers,
Alistair


On 30 June 2017 at 16:42, Alistair Grant <[hidden email]> wrote:
> Hi Stef,
>
> Good pickup.
>
> I also can't load
> SLICE-Issue-20165-Support-segment-path-printing-AlistairGrant.1.
> Somehow it is dependent on FileSystem-Core-AlistairGrant.223, and I
> have no idea what that is (I don't remember ever creating it, and I
> don't have a copy on my machine anywhere, or in any of my backups).
>
> Fortunately I do have a changeset of the patch.  I'll send it through
> as an attachment after I've sent this email in case it causes the
> response to be blocked.
>
> Do you or Pavel have a preference on how to get the PR fixed?  I'm
> happy to generate a new PR if you or Pavel can close the existing one
> (#126).
>
> Thanks,
> Alistair
>
>
> On 30 June 2017 at 16:09, Stephane Ducasse <[hidden email]> wrote:
>> Hi alistair
>>
>> I'm starting to review code that is in the PR pipeline.
>> https://github.com/pharo-project/pharo/pull/126
>>
>> I noticed that you wrote in the slice
>>
>> Changes since last slice:
>>
>> - Change Path>>fullName to just print the path
>> - Fix absolute path strings
>> - Bug fix Path class>>from:delimiter:
>> - Add unit tests
>>
>> But I only see in the PR three methods.
>> Can you check because I have the impression that we are losing code somewhere?
>>
>> BTW I could not see the contents of the AlistairGrant.1 package
>> I tried on several images 70 and 60 latest
>>
>> Stef
>>

Reply | Threaded
Open this post in threaded view
|

Re: About Fogbugz 20165 Support-segment-path-printing

alistairgrant
On Fri, Jun 30, 2017 at 04:55:29PM +0200, Pavel Krivanek wrote:
> The PR is updated
>
> -- Pavel

Thanks, Pavel!



> 2017-06-30 16:44 GMT+02:00 Alistair Grant <[hidden email]>:
>
>     Attached is the changeset as discussed in my last email.
>
>     Just to be clear, this is just for reference, I'm happy to redo the PR
>     if it will save you and/or Pavel some time.
>
>     Cheers,
>     Alistair
>
>
>     On 30 June 2017 at 16:42, Alistair Grant <[hidden email]> wrote:
>     > Hi Stef,
>     >
>     > Good pickup.
>     >
>     > I also can't load
>     > SLICE-Issue-20165-Support-segment-path-printing-AlistairGrant.1.
>     > Somehow it is dependent on FileSystem-Core-AlistairGrant.223, and I
>     > have no idea what that is (I don't remember ever creating it, and I
>     > don't have a copy on my machine anywhere, or in any of my backups).
>     >
>     > Fortunately I do have a changeset of the patch.  I'll send it through
>     > as an attachment after I've sent this email in case it causes the
>     > response to be blocked.
>     >
>     > Do you or Pavel have a preference on how to get the PR fixed?  I'm
>     > happy to generate a new PR if you or Pavel can close the existing one
>     > (#126).
>     >
>     > Thanks,
>     > Alistair
>     >
>     >
>     > On 30 June 2017 at 16:09, Stephane Ducasse <[hidden email]>
>     wrote:
>     >> Hi alistair
>     >>
>     >> I'm starting to review code that is in the PR pipeline.
>     >> https://github.com/pharo-project/pharo/pull/126
>     >>
>     >> I noticed that you wrote in the slice
>     >>
>     >> Changes since last slice:
>     >>
>     >> - Change Path>>fullName to just print the path
>     >> - Fix absolute path strings
>     >> - Bug fix Path class>>from:delimiter:
>     >> - Add unit tests
>     >>
>     >> But I only see in the PR three methods.
>     >> Can you check because I have the impression that we are losing code
>     somewhere?
>     >>
>     >> BTW I could not see the contents of the AlistairGrant.1 package
>     >> I tried on several images 70 and 60 latest
>     >>
>     >> Stef
>     >>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: About Fogbugz 20165 Support-segment-path-printing

Stephane Ducasse-3
In reply to this post by Pavel Krivanek-3
Thanks!
This is cool
I'm learning how to PR too :).

Stef

On Fri, Jun 30, 2017 at 4:55 PM, Pavel Krivanek
<[hidden email]> wrote:

> The PR is updated
>
> -- Pavel
>
> 2017-06-30 16:44 GMT+02:00 Alistair Grant <[hidden email]>:
>>
>> Attached is the changeset as discussed in my last email.
>>
>> Just to be clear, this is just for reference, I'm happy to redo the PR
>> if it will save you and/or Pavel some time.
>>
>> Cheers,
>> Alistair
>>
>>
>> On 30 June 2017 at 16:42, Alistair Grant <[hidden email]> wrote:
>> > Hi Stef,
>> >
>> > Good pickup.
>> >
>> > I also can't load
>> > SLICE-Issue-20165-Support-segment-path-printing-AlistairGrant.1.
>> > Somehow it is dependent on FileSystem-Core-AlistairGrant.223, and I
>> > have no idea what that is (I don't remember ever creating it, and I
>> > don't have a copy on my machine anywhere, or in any of my backups).
>> >
>> > Fortunately I do have a changeset of the patch.  I'll send it through
>> > as an attachment after I've sent this email in case it causes the
>> > response to be blocked.
>> >
>> > Do you or Pavel have a preference on how to get the PR fixed?  I'm
>> > happy to generate a new PR if you or Pavel can close the existing one
>> > (#126).
>> >
>> > Thanks,
>> > Alistair
>> >
>> >
>> > On 30 June 2017 at 16:09, Stephane Ducasse <[hidden email]>
>> > wrote:
>> >> Hi alistair
>> >>
>> >> I'm starting to review code that is in the PR pipeline.
>> >> https://github.com/pharo-project/pharo/pull/126
>> >>
>> >> I noticed that you wrote in the slice
>> >>
>> >> Changes since last slice:
>> >>
>> >> - Change Path>>fullName to just print the path
>> >> - Fix absolute path strings
>> >> - Bug fix Path class>>from:delimiter:
>> >> - Add unit tests
>> >>
>> >> But I only see in the PR three methods.
>> >> Can you check because I have the impression that we are losing code
>> >> somewhere?
>> >>
>> >> BTW I could not see the contents of the AlistairGrant.1 package
>> >> I tried on several images 70 and 60 latest
>> >>
>> >> Stef
>> >>
>
>