[Pharo-project] nil key in last literal in instance side methods (reproducible)
Hi guys. So we have a case where we can reproduce the problem. See http://code.google.com/p/pharo/issues/detail?id=6644 If you take a Pharo 20274 and load this slice: SLICE-Issue-6635-Removing-class-does-not-remove-class-tag-CamilloBruni.1 , the #testInstanceSideMethodsWithNilKeyInLastLiteral starts to fail and we have lots of instance side methods with nil in the last literal (in fact, from the package that the slide installs). I tried to debug the #basicLoad and MC friends to see if I come to the real problem but no luck so far. If anyone wants to take a look, it would be great since I think this is a really important bug. Thanks, -- Mariano http://marianopeck.wordpress.com
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it. Nicolas 2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
Hi guys. So we have a case where we can reproduce the problem. See http://code.google.com/p/pharo/issues/detail?id=6644
If you take a Pharo 20274 and load this slice: SLICE-Issue-6635-Removing-class-does-not-remove-class-tag-CamilloBruni.1 , the #testInstanceSideMethodsWithNilKeyInLastLiteral starts to fail and we have lots of instance side methods with nil in the last literal (in fact, from the package that the slide installs). I tried to debug the #basicLoad and MC friends to see if I come to the real problem but no luck so far. If anyone wants to take a look, it would be great since I think this is a really important bug.
Thanks,
-- Mariano http://marianopeck.wordpress.com
On Thu, Sep 6, 2012 at 9:45 PM, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it.
I am not sure if the changes in this slice are important. I think it just happens that for certain reason, we have this problem of the last literal.
Nicolas
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
Hi guys. So we have a case where we can reproduce the problem. See http://code.google.com/p/pharo/issues/detail?id=6644
If you take a Pharo 20274 and load this slice: SLICE-Issue-6635-Removing-class-does-not-remove-class-tag-CamilloBruni.1 , the #testInstanceSideMethodsWithNilKeyInLastLiteral starts to fail and we have lots of instance side methods with nil in the last literal (in fact, from the package that the slide installs). I tried to debug the #basicLoad and MC friends to see if I come to the real problem but no luck so far. If anyone wants to take a look, it would be great since I think this is a really important bug.
Thanks,
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
On Thu, Sep 6, 2012 at 9:45 PM, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it.
I am not sure if the changes in this slice are important. I think it just happens that for certain reason, we have this problem of the last literal.
I just can't tell if it would help or not, without diff I'm totally blind ;) But sure, a correct code can uncover a bug elsewhere...
Nicolas
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
Hi guys. So we have a case where we can reproduce the problem. See http://code.google.com/p/pharo/issues/detail?id=6644
If you take a Pharo 20274 and load this slice: SLICE-Issue-6635-Removing-class-does-not-remove-class-tag-CamilloBruni.1 , the #testInstanceSideMethodsWithNilKeyInLastLiteral starts to fail and we have lots of instance side methods with nil in the last literal (in fact, from the package that the slide installs). I tried to debug the #basicLoad and MC friends to see if I come to the real problem but no luck so far. If anyone wants to take a look, it would be great since I think this is a really important bug.
Thanks,
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
On 2012-09-06, at 22:26, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
On Thu, Sep 6, 2012 at 9:45 PM, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it.
I am not sure if the changes in this slice are important. I think it just happens that for certain reason, we have this problem of the last literal.
I just can't tell if it would help or not, without diff I'm totally blind ;) But sure, a correct code can uncover a bug elsewhere...
Sorry I don't get it? where do you want a diff? on the google code website? -> we thought once about writing an external website doing that - list the changes - list the results from code review - list the test results basically each bugfix should result in a "review" image which can be downloaded and checked... but then again, somebody has to write it...
2012/9/7 Camillo Bruni <camillobruni@gmail.com>:
On 2012-09-06, at 22:26, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
On Thu, Sep 6, 2012 at 9:45 PM, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it.
I am not sure if the changes in this slice are important. I think it just happens that for certain reason, we have this problem of the last literal.
I just can't tell if it would help or not, without diff I'm totally blind ;) But sure, a correct code can uncover a bug elsewhere...
Sorry I don't get it? where do you want a diff? on the google code website? -> we thought once about writing an external website doing that - list the changes - list the results from code review - list the test results
basically each bugfix should result in a "review" image which can be downloaded and checked...
but then again, somebody has to write it...
No, I'm just suggesting a poor man's simple diff in a mailing list, like squeak trunk. -1) inbox diff : you open a review to the whole community for comments, -2) commit phase (whatever the checks you perform now are unchanged, except you have to take above review into account) -3) trunk diff : you open a last chance review (or post mortem analysis) to retract/correct commits It's totally informal, and why would it work? You have to answer this question first: - how many people have ubiquitous web access to read the code and qualified enough to emit a useful comment? - how many people are downloading an image, updating to some version, opening a MC browser, browsing the diffs? You are proposing a much more ellaborated process to review code before integration. I agree, this information belongs to issue tracker. But formalizing and programming this process is involving (think about concurrent solution, roles, requests/answers timelines etc...) As you said, who will do it?
From pragmatic POV, I prefer a light process that provides some help, than no help at all (which is close to what you get with current state of issue tracker). The code producing the diff and sendig the mail already exist, (you'll have to handle SLICE though)... Bonus: it's Smalltalk code.
If you prefer to wait for something better, then let's wait... Nicolas
previously: [Pharo-project] nil key in last literal in instance side methods (reproducible)
No, I'm just suggesting a poor man's simple diff in a mailing list, like squeak trunk. -1) inbox diff : you open a review to the whole community for comments,
ok, why not on a separate mailing list that might make sense
-2) commit phase (whatever the checks you perform now are unchanged, except you have to take above review into account) -3) trunk diff : you open a last chance review (or post mortem analysis) to retract/correct commits
that is (almost) up and running with our github export: https://github.com/PharoProject/pharo-core
It's totally informal, and why would it work? You have to answer this question first: - how many people have ubiquitous web access to read the code and qualified enough to emit a useful comment? - how many people are downloading an image, updating to some version, opening a MC browser, browsing the diffs?
You are proposing a much more ellaborated process to review code before integration. I agree, this information belongs to issue tracker. But formalizing and programming this process is involving (think about concurrent solution, roles, requests/answers timelines etc...) As you said, who will do it?
From pragmatic POV, I prefer a light process that provides some help, than no help at all (which is close to what you get with current state of issue tracker). The code producing the diff and sendig the mail already exist, (you'll have to handle SLICE though)... Bonus: it's Smalltalk code.
If you prefer to wait for something better, then let's wait...
well the solution would be to simply switch to git for the whole version management. github.com provides an excellent example of how to deal with you aforementioned issues. - pull requests - single commits - full history plus - everything can be commented / annotated from within the website => 0 setup needed so for me something like github is the future for development.
On 7 September 2012 11:52, Camillo Bruni <camillobruni@gmail.com> wrote:
previously: [Pharo-project] nil key in last literal in instance side methods (reproducible)
No, I'm just suggesting a poor man's simple diff in a mailing list, like squeak trunk. -1) inbox diff : you open a review to the whole community for comments,
ok, why not on a separate mailing list that might make sense
-2) commit phase (whatever the checks you perform now are unchanged, except you have to take above review into account) -3) trunk diff : you open a last chance review (or post mortem analysis) to retract/correct commits
that is (almost) up and running with our github export: https://github.com/PharoProject/pharo-core
It's totally informal, and why would it work? You have to answer this question first: - how many people have ubiquitous web access to read the code and qualified enough to emit a useful comment? - how many people are downloading an image, updating to some version, opening a MC browser, browsing the diffs?
You are proposing a much more ellaborated process to review code before integration. I agree, this information belongs to issue tracker. But formalizing and programming this process is involving (think about concurrent solution, roles, requests/answers timelines etc...) As you said, who will do it?
From pragmatic POV, I prefer a light process that provides some help, than no help at all (which is close to what you get with current state of issue tracker). The code producing the diff and sendig the mail already exist, (you'll have to handle SLICE though)... Bonus: it's Smalltalk code.
If you prefer to wait for something better, then let's wait...
well the solution would be to simply switch to git for the whole version management. github.com provides an excellent example of how to deal with you aforementioned issues.
- pull requests - single commits - full history
plus - everything can be commented / annotated from within the website => 0 setup needed
so for me something like github is the future for development.
And in answer to Nicolas' comment about ubiquitous web access, GitHub can mail interested parties when something interesting happens - a new issue, issue state change, new comments - and you can reply to those via email. frank
That would be much much better than what we currently have with mix of code agnostic google issue tracker + ST only MC repository... Another plus is also to make code visible/searchable/indexable outside our community, be it with less specialized (not so less powerfull) IDE. Nicolas 2012/9/7 Frank Shearar <frank.shearar@gmail.com>:
On 7 September 2012 11:52, Camillo Bruni <camillobruni@gmail.com> wrote:
previously: [Pharo-project] nil key in last literal in instance side methods (reproducible)
No, I'm just suggesting a poor man's simple diff in a mailing list, like squeak trunk. -1) inbox diff : you open a review to the whole community for comments,
ok, why not on a separate mailing list that might make sense
-2) commit phase (whatever the checks you perform now are unchanged, except you have to take above review into account) -3) trunk diff : you open a last chance review (or post mortem analysis) to retract/correct commits
that is (almost) up and running with our github export: https://github.com/PharoProject/pharo-core
It's totally informal, and why would it work? You have to answer this question first: - how many people have ubiquitous web access to read the code and qualified enough to emit a useful comment? - how many people are downloading an image, updating to some version, opening a MC browser, browsing the diffs?
You are proposing a much more ellaborated process to review code before integration. I agree, this information belongs to issue tracker. But formalizing and programming this process is involving (think about concurrent solution, roles, requests/answers timelines etc...) As you said, who will do it?
From pragmatic POV, I prefer a light process that provides some help, than no help at all (which is close to what you get with current state of issue tracker). The code producing the diff and sendig the mail already exist, (you'll have to handle SLICE though)... Bonus: it's Smalltalk code.
If you prefer to wait for something better, then let's wait...
well the solution would be to simply switch to git for the whole version management. github.com provides an excellent example of how to deal with you aforementioned issues.
- pull requests - single commits - full history
plus - everything can be commented / annotated from within the website => 0 setup needed
so for me something like github is the future for development.
And in answer to Nicolas' comment about ubiquitous web access, GitHub can mail interested parties when something interesting happens - a new issue, issue state change, new comments - and you can reply to those via email.
frank
On
No, I'm just suggesting a poor man's simple diff in a mailing list, like squeak trunk. -1) inbox diff : you open a review to the whole community for comments, -2) commit phase (whatever the checks you perform now are unchanged, except you have to take above review into account) -3) trunk diff : you open a last chance review (or post mortem analysis) to retract/correct commits
It's totally informal, and why would it work? You have to answer this question first: - how many people have ubiquitous web access to read the code and qualified enough to emit a useful comment? - how many people are downloading an image, updating to some version, opening a MC browser, browsing the diffs?
You are proposing a much more ellaborated process to review code before integration. I agree, this information belongs to issue tracker. But formalizing and programming this process is involving (think about concurrent solution, roles, requests/answers timelines etc...) As you said, who will do it?
From pragmatic POV, I prefer a light process that provides some help, than no help at all (which is close to what you get with current state of issue tracker). The code producing the diff and sendig the mail already exist, (you'll have to handle SLICE though)... Bonus: it's Smalltalk code.
If you prefer to wait for something better, then let's waitâ¦
is there a setting in ss3 that can do that? How this is done in squeak?
Nicolas
thanks to raise this issue again. So what concretely do you think would help? Emitting a diff? and sending it to the mailing-list stef On Sep 6, 2012, at 9:45 PM, Nicolas Cellier wrote:
A bot providing simple diffs would help more eyes reviewing code... By now this important work is restricted to a narrow team of busy active developpers. Of course, CI may help a bit, but monkeys don't read code they just evaluate it.
Nicolas
2012/9/6 Mariano Martinez Peck <marianopeck@gmail.com>:
Hi guys. So we have a case where we can reproduce the problem. See http://code.google.com/p/pharo/issues/detail?id=6644
If you take a Pharo 20274 and load this slice: SLICE-Issue-6635-Removing-class-does-not-remove-class-tag-CamilloBruni.1 , the #testInstanceSideMethodsWithNilKeyInLastLiteral starts to fail and we have lots of instance side methods with nil in the last literal (in fact, from the package that the slide installs). I tried to debug the #basicLoad and MC friends to see if I come to the real problem but no luck so far. If anyone wants to take a look, it would be great since I think this is a really important bug.
Thanks,
-- Mariano http://marianopeck.wordpress.com
participants (5)
-
Camillo Bruni -
Frank Shearar -
Mariano Martinez Peck -
Nicolas Cellier -
Stéphane Ducasse