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 |
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 > |
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 |
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. |
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 > >> > > |
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 >> >> > > |
Free forum by Nabble | Edit this page |