search

codingteam / pacmacs.el

Pacman for Emacs
Other
92 stars 9 forks source link

Implement terrified ghost #160

Closed rexim closed 8 years ago

rexim commented 8 years ago

Should close #153

rexim commented 8 years ago

Wow! It's nice to have the coverage check not passed! :) I gotta write some unit tests then.

rexim commented 8 years ago

@ForNeVeR, please reassign this PR on me when you are done with review. I'll merge it by myself when I'm done with manual testing on both platforms. Thanks!

rexim commented 8 years ago

Hm... I just thought that I would like to fix the coverage check first. I want to generalize some functions to decrease the amount of code lines and probably completely remove something. And I don't wanna interfere with your review Mr @ForNeVeR. So I reassign this PR to myself until I fix the coverage check and do the manual testing. Thanks!

rexim commented 8 years ago

Well, since you started to review, reassigning the PR back to you :)

ForNeVeR commented 8 years ago

Overall I'd say that the pacmacs.el is getting huge, so maybe you need more modules here.

I'd suggest to move all the object management code to one file, state management to another, and (that's the most important part) emacs environment interoperation (such as input management and rendering) to the third file. It will make it more easy to create a server implementation in future.

But for now it is not obligatory to make the changes I mention in this comment; it's up to you at the moment.

rexim commented 8 years ago

@ForNeVeR good point! I think we need to create an issue for that with refactor tag. I'll do that.

Thank you very much Mr @ForNeVeR for your review. :)

ForNeVeR commented 8 years ago

Yeah, I've finished with it.

rexim commented 8 years ago

Average tick time has been increased on Linux from 8ms to 9ms, which is fine for me.

rexim commented 8 years ago

But on Windows it decreased from 29ms to 24ms! What a hell! Well, I'm fine with that too. :)

Anyway, everything works fine on Windows. I'm mergin the request. Thanks!