View Single Post
  #7  
Old 13th January 2015, 06:05 PM
HatCat's Avatar
HatCat HatCat is offline
Alpha Tester
Project Supporter
Senior Member
 
Join Date: Feb 2007
Location: In my hat.
Posts: 16,256
Default

Nothing personal, but that is one pull request I would not want to accept in any repo.

See, this is a challenging aspect of open-source: Organization. You must be very detailed to have an organized, documented tree of source feedback. If you don't, then it's just, "My code is improved over your code; therefore you should merge it." If not done healthily, it's possible for open-source to expose more arrogance than what could have happened in closed-source. (That being said, it's a moot discussion mostly since zilmar only published the public Git repository since there is no longer a need for the private one anyway at this point, not because he's targeting open-source principles.)

If you look at that PR you'll see commits of various types (compatibility fixes, optimizations, accuracy "changes") all mixed together in one pull request. I think zilmar shouldn't merge that--not because the changes are bad, but because it's too ambiguous. He cannot verify the solidity of all of those changes of different types in an efficient amount of time, and there is NO explanation on the PR.

I recommend taking it slower! When I started sending pull requests I did maybe 1 or 2 commit at a time, both of strongly related commit reasons (like easy compiler warning fixes), and avoided changing too much code. The more code you change in each single pull request, the more challenging it will be for Mr. zilmar to understand the safety and equivalence of that algorithm to the stability of the original one. If fixing/improve stability you should comment a lot about that in the PR and not use lazy commit names.
Reply With Quote