yavl / teeworlds-infclassR

Slightly modified version of official InfClass
Other
10 stars 4 forks source link

How to proceed with development? #157

Closed teoman002 closed 5 years ago

teoman002 commented 5 years ago

Quote from Breton:

This will be the third commit related to initial infection that we have to revert or heavily change. All three were heavy rewrites. All three were not reviewed. We should stop doing huge changes in order to fix bugs. Fixes should address one specific issue at a time, be small and reviewable. Nobody will review and test rewrites, including authors of the rewrites.

My fix was necessary because the initial infection mechanic always ended in people getting infected unfairly. Furthermore people were infected not randomly. Since I am the only one working on the high priority initial infection bug I had the feeling that it is important to do the infection process differently.

I already wrote in another thread about it, what I planned to do, but no one was answering if that is okay or that I should stop some days because some of us might solve it with a one liner.

We should stop doing huge changes in order to fix bugs.

If you want to have the bug fixed by me, you have to deal with it, or do it yourself. Not every bug is a one liner (#127 ,#130) , likewise not every bug needs a rewrite.

Fixes should address one specific issue at a time, be small and reviewable.

I understand why, and I do agree with you. For the infection rewrite some methods from an pending Pull Request ( #138) were obligatory, to write the initial infection like that.

Nobody will review and test rewrites, including authors of the rewrites.

I don't understand how you allege me of not testing this rewrite properly. If you code something, bugs can happen besides this fix is stable, it does compile and doesn't crash. I played with 5 players today, without them complaining about anything.

As I understand you want it like that:

Fixes should address one specific issue at a time, be small and reviewable.

I had the problem of having the tools writing the high priority fix not at hand. Therefore it was time to merge them. If you want I divide them next time into the tool merge and later do the fix. Consequently you would see the pull request merge, which was big. And you would see the fix, which was also big. But in the end they would be smaller and you might be able to understand what is going on.

My question to all developers would be this: Do you need to understand every change another developer has made code wise or is a summary of changes sufficient? Personally I don't have the need to review what someone else did, as long as it does not kill the server. Bugs can I.m.o. always be fixed afterwards.