meebey / smuxi

Smuxi is an user-friendly and free IRC client for Linux, Windows and Mac OS X based on GNOME / GTK+
https://smuxi.im/
GNU General Public License v2.0
172 stars 46 forks source link

Refactoring and warnings #226

Closed knocte closed 7 years ago

knocte commented 9 years ago

Fix warnings, typos and do some maintainability refactoring.

knocte commented 9 years ago

ping :)

PythonProdigy commented 8 years ago

@knocte I wish you the best of luck on getting a reply, hopefully my depressing comment will help with that. @meebey can be quite disappointing when it comes to merging PRs and he seems to have a bad habit of letting issues like these get stale. I have a good friend who submitted many fixes and full web socket support to his IRC library and it's been rotting away for a while with nary a second thought.

This is a topic for another issue but it's times like these I feel a deep sense of sadness rush over me when it comes to the treatment of women in the tech field. As a woman software developer myself I refuse to debate discrimination, harassment or unequal treatment of pull request prioritization on GitHub but it's unfortunately a real issue for a lot of repo maintainers.

meebey commented 8 years ago

@PythonProdigy @knocte got my reply and my concerns still hold. Refactoring code is what needs most attention to make sure it does not introduce regressions and unneeded code changes need to be avoided at all cost (to aim for stability). I have no idea on what base you make that statement about PRs getting stale. Since @knocte is male and not female, PRs can get stale independent on the gender as you asserted but based on the changes in the PR.

PS: this PR this not fixing any issue and it also does not add any feature, thus it has a low priority. But as mentioned it breaks compat with smuxi-servers, since MessageBuilder is not [Serializable]

knocte commented 7 years ago

@PythonProdigy sorry for the late reply. I've been busy with... life.

While I can see that your message comes with the best intentions, I think it was a bit misguided and actually wrong. First, because, as @meebey already said, I got a reply, from him after I sent the "ping" (dated Aug 14, 2015), but I guess it's difficult to see because he replied on the spot of the diff that we were discussing about (dated Dec 17, 2015).

On top of that, I'm not female, so your feminist paranoias wouldn't apply here.

Last but not least, I actually watch the library that @meebey also maintains to which your friend contributed to. If I were the maintainer of the library, I would have acted like him: requesting to improve the changes. I remember the pull requests that your friend presented where plagued with unrelated changes in each commit, which made it very difficult to review.

I know, this might sound like another guy that is not on your side. But really, polishing open source development skills takes time. Pull requests need to come with a specific purpose and not deviate a single bit from it. If the pull request fixes 2 unrelated issues, it's already wrong. So imagine if each commit of the pull request contains unexplained changes for many things... Impossible to review. And normal that it gets nowhere.

I would be happy to help by mentoring you folks, if you ever need the help. Cheers, happy hacking!

knocte commented 7 years ago

(Edited my comment above to fix typo in a date)

knocte commented 7 years ago

Closing, because:

  1. https://github.com/meebey/smuxi/pull/226/commits/68dbda6f4f0dd570388d635d85ba086c61c8c2e0 was already committed separately in master: https://github.com/meebey/smuxi/commit/fe5471a3dbdf975b08ff00c5b61085b3949b33a3
  2. https://github.com/meebey/smuxi/pull/226/commits/3e41603c901d59469de8ece335763ba64a688ba7 was also committed separately in master: https://github.com/meebey/smuxi/commit/9194d47a55514fbe7080b8377010fa46fe193589
  3. Rest of the content of this pull request has been superseded by this other pull request: https://github.com/meebey/smuxi/pull/258