Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colorama communicator restructuring #52

Merged
merged 4 commits into from
Jan 3, 2015
Merged

Colorama communicator restructuring #52

merged 4 commits into from
Jan 3, 2015

Conversation

jovanbulck
Copy link
Collaborator

DialogCommunicator now handles the eventSamenvatting:

screenshot from 2014-12-29 11 51 22

Sorry for the trailing space removal of atom again. I'll have to look into this next time...

I's a quick hack however. See last comment in issue #49 for a proposed alternative

@GijsTimmers
Copy link
Owner

OK, thank you for taking the time in this busy period. eventSamenvattingGeven() is not a real necessity at the moment, so if this is just a quick hack, we should consider refactoring the code altogether, as you proposed in #49.

@jovanbulck
Copy link
Collaborator Author

Ok 👍

We'll see then when implementing the factory pattern in #49 . The general idea of this pull request, using self.msg to display additional msg in the dialog is ok though. So that should easily be integrated in a LoginDialogCommunicator and/or LogoutDialogCommunicator

@jovanbulck
Copy link
Collaborator Author

@GijsTimmers Apparently I pushed into this open pull request, but the original eventSamenvatting is gone so it's ok I think...

  • Added some small comment questions in communicator.py
  • minor enhancement to the bubblecommunicator: more elegant createAndShowNotification method in superclass
  • major restructuring of the plaintext and colorama communicator to avoid code duplication and facilitate overriding appearance Check it out
  • changed some lines in kotnetcli.py in order to be able to test; you should maybe consider a dev branch, so the master remains working ;-)

@jovanbulck jovanbulck changed the title eventSamenvatting voor dialogCommunicator Colorama communicator restructuring Jan 2, 2015
@GijsTimmers GijsTimmers merged commit a3ce3cc into GijsTimmers:master Jan 3, 2015
@GijsTimmers
Copy link
Owner

Please checkout to the dev branch for work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants