search

ccw-ide / ccw

Counterclockwise is an Eclipse plugin helping developers write Clojure code
https://github.com/laurentpetit/ccw/wiki/GoogleCodeHome
Eclipse Public License 1.0
220 stars 50 forks source link

Async exec refactoring #815

Closed arichiardi closed 9 years ago

arichiardi commented 9 years ago

Hello Laurent, I was really really bothered by the tests not succeeding and therefore I refactored all the .asyncExec functions in one place (DisplayUtil) so that we can check for null and isDisposed before executing any asyncExec/syncExec code on Display.

I hope I did not break anything and I still need to check that all the functionalities are working (especially the Outline). In the meantime I am putting this PR up so that I can check if Travis has green tests like me on my machine.

I also would like to ask you some question in the code that I refactored.

:+1:

arichiardi commented 9 years ago

Sorry for the current mess, I have not yet rebased on master, I will do it.

arichiardi commented 9 years ago

Great this is green again! I would like to add the SWTBot test generator as part of the menu, like in here under Programatically, from a plugin, only of course maybe if ccw is launched with -debug. What do you think? And maybe while testing this addition I will also add some tests to SmokeTests :smile:

arichiardi commented 9 years ago

Please disregard the first two commits, I am going to remove them as they are not part of this PR.

arichiardi commented 9 years ago

This one should be ready, but I would like to merge the swtbot-generator branch first so that I can work on some tests here...ok?

laurentpetit commented 9 years ago

Ok but please not in master. I'd like to prepare a release and stabilize things ! :-)

— Envoyé avec Mailbox

On Fri, Jul 17, 2015 at 7:16 PM, Andrea Richiardi notifications@github.com wrote:

This one should be ready, but I would like to merge the swtbot-generator branch first so that I can work on some tests here...ok?

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/pull/815#issuecomment-122346346

arichiardi commented 9 years ago

Sure thing! Of course I will be waiting your approval :wink:

arichiardi commented 9 years ago

Hello Laurent, what about this PR, have you had time to have a look at it? It would also solve the SmokeTests issue and we would have all green again..

laurentpetit commented 9 years ago

Sure I need to review it, it was not on my radar, sorry

arichiardi commented 9 years ago

:+1: Thanks, just because I was taking up coding the partitions and I noticed this (that I refactored out from there) was not in yet... :smile:

laurentpetit commented 9 years ago

If you don't mind, I will first test commit https://github.com/laurentpetit/ccw/commit/1e3819e5257fce4af0ee0dd9d5f7d0c4d1121bf2 and add (after some bed time) the additional modifications I brainstormed on the comments.

I don't understand everything in the second commit (related to SmokeTests), but this can be done as a separate work not preventing me from integrating the work on ui.

laurentpetit commented 9 years ago

we'll keep the PR around for the time being, as it includes interesting comments/discussions

arichiardi commented 9 years ago

Ok I saw the other PR, I will probably just integrate the second commit in swtbot-generator, the one that is more about tests...which BTW can be merged as far as it is all green :+1: I am planning to reintroduce your mistakenly removed test there as well don't worry :smile:

laurentpetit commented 9 years ago

(y) ok. So as soon as I've done a new pass across the comments on this PR, I will close it.