search

flowdock / oulu

Flowdock IRC Gateway
MIT License
76 stars 16 forks source link

Render edit message #27

Closed OsQu closed 11 years ago

OsQu commented 11 years ago

Render edit message event by appending asterisk to the edited message.

19:26 < Oskari> Plul request!
19:26 < Oskari> Pull request!*
19:26 < Oskari> [Pull request!] << which I coment..
19:26 < Oskari> [Pull request!] << which I comment..*
anttipitkanen commented 11 years ago

As discussed in the planning session, the message-edit rendering should have some time limit. As we can soon edit any message in the history, the IRC should not render every message-edit event it receives. Instead, it should render only the ones that are for recent messages. Again, not the most trivial thing to implement, but it's definitely something that is needed for this to work. :)

OsQu commented 11 years ago

Aah gotcha and now that I think of it, it does ring the bell :P

I'll add some configurable time limit.

OsQu commented 11 years ago

Did it a bit better this time.

When edit-message-event arrives, fetch message data from REST API and act accordingly. Default time for rendering edit event is 10 minutes. (hard coded constant)

OsQu commented 11 years ago

On a second thought, 10 minutes is way too long since this is meant mainly for typo fixing.. Probably something like 30-60 seconds is more suitable.

anttipitkanen commented 11 years ago

The ApiHelper class is neat! One suggestion: instead of creating new instances of it in several places (with the email/pass attributes), could the IrcConnection just have one instance of the ApiHelper that is pre-configured with the correct credentials?

OsQu commented 11 years ago

The problem with shared instance is that it would turn immutable object to a mutable one. Irc connection is initialised before authentication, where we get user's credentials. So either you have to do some kind of authenticate method which fills the parameters (and throw an error if user tries to use the helper before authentication) after authentication. Or initialise the helper when we've authenticated in which case you'll get undefined erros when trying to use it before authenticating, which would require null-guards.

anttipitkanen commented 11 years ago

Ah, didn't check that. I guess it's then good to leave it as it is.

anttipitkanen commented 11 years ago

Let's test this in the QA environment then.