search

TOGoS / TMCMR

TOGoS's Minecraft Map Renderer
http://www.nuke24.net/projects/TMCMR/
Other
45 stars 18 forks source link

Adding a GUI (2) #34

Closed piegamesde closed 7 years ago

piegamesde commented 7 years ago

Here my second try, but this time with cleaner pull requests.

piegamesde commented 7 years ago

No it isn't entirely. There are a couple -- very few -- changes that were made to the code. Unfortunately, I didn't disable automatic code formatting on saving, that's why they somehow get lost.

I also don't know why it isn't able to merge anymore -- yesterday everything was fine.

piegamesde commented 7 years ago

Is there any reason why this has not been merged yet? Anything I should change? I won't be able to keep up with the main branch forever.

TOGoS commented 7 years ago

06446bb goes above and beyond my usual reasons for avoiding unnecessary formatting changes. It screws up alignment of a bunch of code and randomizes the newlines in a bunch of documentation blocks. In order to merge this I'm going to have to go through and understand what your changes are for so I can pick out the ones that make sense, and I haven't had that much free time. This is why I asked you resubmit with only the necessary changes.

piegamesde commented 7 years ago

I reverted the changes in 06446bb with 63f3d44 and redid them in 42ff1ec without the formatting changes.

If you look at the changed files in this PR, you will see that most of the changes are in new files, except for three files where 2, 9 and 110 lines got changed.

piegamesde commented 7 years ago

Apparently this is how you revert commits on GitHub - create a new commit that reverts all the changes. If you are still worried these changes could ruin the history of the file (I doubt so since if you look at the file diff of this PR they don't show up), I can redo this pull request from scratch without this mistake. If it is because the changes in RegionRenderer don't fit the previous formatting that prevents you from merging, just tell it.

I spent over three months on this feature, those 30 minutes of extra work won't stop me. So if you still don't like something about my changes, tell me and I'll fix it.

TOGoS commented 7 years ago

That is one way to do it. You can do a git rebase -i <base commit ID> and shuffle things around with "squash" and "fixup" (I use "fixup" a lot when I'm trying to make it look like I never introduced a bug on my branch that I later fixed).

Git's a funny thing. You can use it to track the actual history of all the changes anyone ever made, but as far as my personal feature branches are concerned I find it more useful for tracking what history should have been. i.e. 1) refactor, 2) infrastructure for feature, 3) feature. If after (3) I realize I introduced bugs in (1) or (2) I'll commit the fixes but then do a rebase and turn those into fixups. One reason for this is to tell a more coherent story to anyone else reading through my commits (including my future self). Another is so that if I want to take just one of those changes and apply to another branch, any single commit is in top condition (not containing any known bugs, because I fixupped any bugfixes into it). And another is to make git bisect more useful.

Which is all to say I have reasons for wanting to keep the commit history clean, because I don't want you to think I'm being overly stuck-up-the-butt-with-a-stick. In this case my primary reason is the 'coherent story' one, because I want to know what you actually did before I merge it. :)

TOGoS commented 7 years ago

I did the rebase and made https://github.com/TOGoS/TMCMR/tree/piemaster.