review pull request changes view

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

review pull request changes view

Nicolai Hess-3-2
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

Inline-Bild 1


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?
Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2


2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

Inline-Bild 1


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?



Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2


2017-09-01 15:03 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?




Yes,

attached a another crash.dmp (maybe it helps)


crash.dmp (111K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM

On 1 Sep 2017, at 15:50, Nicolai Hess <[hidden email]> wrote:



2017-09-01 15:03 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?




Yes,

attached a another crash.dmp (maybe it helps)

<crash.dmp>

can you tell me the PR you were analysing ?

Esteban
Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM

On 1 Sep 2017, at 16:02, Esteban Lorenzano <[hidden email]> wrote:


On 1 Sep 2017, at 15:50, Nicolai Hess <[hidden email]> wrote:



2017-09-01 15:03 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?




Yes,

attached a another crash.dmp (maybe it helps)

<crash.dmp>

can you tell me the PR you were analysing ?

ah, and btw… can you update with latest dev-0.5? I made some changes that may help there that I still do not merge into master.

Esteban


Esteban

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by EstebanLM


2017-09-01 16:02 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 15:50, Nicolai Hess <[hidden email]> wrote:



2017-09-01 15:03 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?




Yes,

attached a another crash.dmp (maybe it helps)

<crash.dmp>

can you tell me the PR you were analysing ?

 

Esteban

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by EstebanLM


2017-09-01 16:13 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 16:02, Esteban Lorenzano <[hidden email]> wrote:


On 1 Sep 2017, at 15:50, Nicolai Hess <[hidden email]> wrote:



2017-09-01 15:03 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 14:51, Nicolai Hess <[hidden email]> wrote:



2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

<grafik.png>


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?


BTW, this is extremly unstable, reviewing a pull request, crashes a lot (the vm)

on windows?




Yes,

attached a another crash.dmp (maybe it helps)

<crash.dmp>

can you tell me the PR you were analysing ?

ah, and btw… can you update with latest dev-0.5? I made some changes that may help there that I still do not merge into master.

update what? pharo ?
 

Esteban


Esteban


Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

CyrilFerlicot
On Fri, Sep 1, 2017 at 4:26 PM, Nicolai Hess <[hidden email]> wrote:
>
>
>
> update what? pharo ?
>
>

You can update Iceberg by executing:

#('Iceberg-UI' 'Iceberg-Plugin' 'Iceberg-Metacello-Integration'
'Iceberg-Libgit' 'Iceberg' 'BaselineOfIceberg' 'LibGit-Core'
'BaselineOfLibGit')
do: [ :each | each asPackage removeFromSystem ].

Metacello new
  baseline: 'Iceberg';
  repository: 'github://pharo-vcs/iceberg:dev-0.5';
  load.



--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by Nicolai Hess-3-2


2017-09-01 14:48 GMT+02:00 Nicolai Hess <[hidden email]>:
When reviewing a pull request, I see this in the changes view:
The tab title says
"left working copy / right incoming update"
but the code on the right side is what is current in my image

Inline-Bild 1


And I don't think "working copy" is the correct naming, as this view lets me diff changes within
that history, so it may be that none of this code is actually in my "working copy", no ?

And how do I know if a pull request / change is already included ?

I looked at a pull request
and it seems this change is already in my image (a fresh image from 30.08 (Pharo7.0alpha.build.78.sha.35c21c3fac9b557eb38ec569fbbb109f2015c71a)
The pull request (https://api.github.com/repos/pharo-project/pharo/pulls/231) is  "open", but I can see the new code already in my image.

The diff view tells me, that the method
UITheme>>warningTextColor changes from

warningTextColor
    ^ Color yellow muchDarker

to

warningTextColor
    ^ Color yellow

But the method in the image already looks like this:

warningTextColor
    ^ Color yellow
 

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by CyrilFerlicot


2017-09-01 16:29 GMT+02:00 Cyril Ferlicot <[hidden email]>:
On Fri, Sep 1, 2017 at 4:26 PM, Nicolai Hess <[hidden email]> wrote:
>
>
>
> update what? pharo ?
>
>

You can update Iceberg by executing:

#('Iceberg-UI' 'Iceberg-Plugin' 'Iceberg-Metacello-Integration'
'Iceberg-Libgit' 'Iceberg' 'BaselineOfIceberg' 'LibGit-Core'
'BaselineOfLibGit')
do: [ :each | each asPackage removeFromSystem ].

Metacello new
  baseline: 'Iceberg';
  repository: 'github://pharo-vcs/iceberg:dev-0.5';
  load.



Ah, thanks. I will try it out and report back if the crashes still occur.
 

--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France


Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

CyrilFerlicot
In reply to this post by Nicolai Hess-3-2


On Fri, Sep 1, 2017 at 4:46 PM, Nicolai Hess <[hidden email]> wrote:


And how do I know if a pull request / change is already included ?

I looked at a pull request
and it seems this change is already in my image (a fresh image from 30.08 (Pharo7.0alpha.build.78.sha.35c21c3fac9b557eb38ec569fbbb109f2015c71a)
The pull request (https://api.github.com/repos/pharo-project/pharo/pulls/231) is  "open", but I can see the new code already in my image.

The diff view tells me, that the method
UITheme>>warningTextColor changes from

warningTextColor
    ^ Color yellow muchDarker

to

warningTextColor
    ^ Color yellow

But the method in the image already looks like this:

warningTextColor
    ^ Color yellow
 


The *old* implementation is

warningTextColor
  ^ Color yellow

and the *new* implementation is

warningTextColor
  ^ Color yellow muchDarker


So, the diff may be inversed. (I did not checked)


--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France
Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM

On 1 Sep 2017, at 16:50, Cyril Ferlicot <[hidden email]> wrote:



On Fri, Sep 1, 2017 at 4:46 PM, Nicolai Hess <[hidden email]> wrote:


And how do I know if a pull request / change is already included ?

I looked at a pull request
and it seems this change is already in my image (a fresh image from 30.08 (Pharo7.0alpha.build.78.sha.35c21c3fac9b557eb38ec569fbbb109f2015c71a)
The pull request (https://api.github.com/repos/pharo-project/pharo/pulls/231) is  "open", but I can see the new code already in my image.

The diff view tells me, that the method
UITheme>>warningTextColor changes from

warningTextColor
    ^ Color yellow muchDarker

to

warningTextColor
    ^ Color yellow

But the method in the image already looks like this:

warningTextColor
    ^ Color yellow
 


The *old* implementation is

warningTextColor
  ^ Color yellow

and the *new* implementation is

warningTextColor
  ^ Color yellow muchDarker


So, the diff may be inversed. (I did not checked)

yep. Damn it :)

Esteban



--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by Nicolai Hess-3-2


2017-09-01 16:48 GMT+02:00 Nicolai Hess <[hidden email]>:


2017-09-01 16:29 GMT+02:00 Cyril Ferlicot <[hidden email]>:
On Fri, Sep 1, 2017 at 4:26 PM, Nicolai Hess <[hidden email]> wrote:
>
>
>
> update what? pharo ?
>
>

You can update Iceberg by executing:

#('Iceberg-UI' 'Iceberg-Plugin' 'Iceberg-Metacello-Integration'
'Iceberg-Libgit' 'Iceberg' 'BaselineOfIceberg' 'LibGit-Core'
'BaselineOfLibGit')
do: [ :each | each asPackage removeFromSystem ].

Metacello new
  baseline: 'Iceberg';
  repository: 'github://pharo-vcs/iceberg:dev-0.5';
  load.



Ah, thanks. I will try it out and report back if the crashes still occur.
 

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

Inline-Bild 1


How can I get back the system repository ?
 

--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France



Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

CyrilFerlicot


On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

Inline-Bild 1


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  



--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France
Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2


2017-09-01 17:06 GMT+02:00 Cyril Ferlicot <[hidden email]>:


On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

Inline-Bild 1


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  

yes, seems to work.
 



--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM
In reply to this post by CyrilFerlicot

On 1 Sep 2017, at 17:06, Cyril Ferlicot <[hidden email]> wrote:



On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

<grafik.png>


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  

mmm no. 
Better: 

IceCommitInfo allInstances anyOne repository register.

(assuming anyOne will answer a pharo package)

Esteban




--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France

Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2


2017-09-01 17:11 GMT+02:00 Esteban Lorenzano <[hidden email]>:

On 1 Sep 2017, at 17:06, Cyril Ferlicot <[hidden email]> wrote:



On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

<grafik.png>


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  

mmm no. 
Better: 

IceCommitInfo allInstances anyOne repository register.

(assuming anyOne will answer a pharo package)

hm, no.

"IceCommitInfo allInstances" is empty.

 

Esteban




--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France


Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

Nicolai Hess-3-2
In reply to this post by Nicolai Hess-3-2


2017-09-01 17:09 GMT+02:00 Nicolai Hess <[hidden email]>:


2017-09-01 17:06 GMT+02:00 Cyril Ferlicot <[hidden email]>:


On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

Inline-Bild 1


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  

yes, seems to work.
 

But, is the

GitHub -> Review pull requests ....

menu entry gone? I can not find it anymore.
 



--
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France


Reply | Threaded
Open this post in threaded view
|

Re: review pull request changes view

EstebanLM

On 1 Sep 2017, at 17:19, Nicolai Hess <[hidden email]> wrote:



2017-09-01 17:09 GMT+02:00 Nicolai Hess <[hidden email]>:


2017-09-01 17:06 GMT+02:00 Cyril Ferlicot <[hidden email]>:


On Fri, Sep 1, 2017 at 5:04 PM, Nicolai Hess <[hidden email]> wrote:

Oh no!

Now iceberg shows an empty list. Where is the pharo repository gone ?
It took me a week to be at this point :(

I reenabled the setting "Include system repositories by default", but still, the window is empty:

<grafik.png>


How can I get back the system repository ?

I'm not 100% sure but I think you just need to click on "add local repository", point the pharo clone and add src as subdirectory.  

yes, seems to work.
 

But, is the 

GitHub -> Review pull requests ....

menu entry gone? I can not find it anymore.

nope.
somehow something went really bad with your update.

anyway, it will not solve the crash problem, I just reproduced it (is just random… which makes it worst)

Esteban

 



-- 
Cyril Ferlicot
https://ferlicot.fr

http://www.synectique.eu
2 rue Jacques Prévert 01,
59650 Villeneuve d'ascq France

12