Hi,
I’m new here and I’m looking into contributing to Openfire. I found https://issues.igniterealtime.org/browse/OF-1795. Would it still be useful to implement caching to RosterItem in a similar way as caching is implemented for Roster?
What is your your position on refactoring existing code (mainly improving readability and removing duplicate code) and adding testcases? Is that welcomed?
Thanks,
Sicco
Greetings. Thanks for your interest in helping out. We welcome any and all pull requests! Adding test cases is icing on the cake!
Wasn’t such a cache added already? See https://github.com/igniterealtime/Openfire/pull/1392
Refactoring to improve code quality are more than welcome, especially if they reduce code smell. I am, however, reluctant to add changes that exclusively apply style changes: beauty is in the eye of the beholder. For Openfire, we’re not very strict about this. We use spaces, not tabs, and linux, not windows newlines, but that’s about it. Style preferences change a lot from person to person. If uncertain, I’d prefer to err on the side of ‘retaining a readable diff’ as opposed to ‘having a uniform style’.
Thank you for your observation.
Regarding the caching of RosterItems:
Hadn’t seen PR 1392, but I’ve been exploring the code regarding RosterItem, RosterItemProvider, RosterManager etc. I was nearing the same conclusion.
So, OF-1795 may be closed, then?
Regarding Refactoring:
I’m all for focusing on code quality, not (just) style changes.
Are there other improvements/requests that are open for Roster and related classes? I’m a bit more familiar with that area now. Otherwise I’ll browse for something suitable.
Regards
The issue might have remained open for a to-be-applied improvement, I’m not sure.
I’m very bad at remembering what issues are still open, but nothing pops to mind. You might as well pop in our chat room (open_chat@conference.igniterealtime.org). As far as we do coordination, it mostly happens there. 