Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Roadmap to NBT 2.0 #55

Open
macfreek opened this Issue Apr 19, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Collaborator

macfreek commented Apr 19, 2012

@stumpylog @twoolie Hey Trenton, I see your active again, which is great. However, some of the changes may introduce some backward incompatibility. So we need to find a balance between fast progress (rapid prototyping) and robustness.

I propose the following:

  • I'll add some test cases to easily check if new features don't break old stuff. Detailed tests for each module are ideal, but for now, I'll settle for a travis test to run all examples with a sample world.
  • As soon as that's done and the first actual changes come up (and this seems the case with stumpylog's 07a7816 commit), I'll create a v2.x branch which should lead to NBT version 2.0.0.

Regular bug fixes and documentation enhancement should take place in the master branch, and should be ported to the v2.x branch (not the other way around please)

This can work, but requires some important coding hygiene:

  • Commit and make pull requests often, in particular for changes that affect a lot of code, like documentation changes.
  • Most development should happen against the master (1.x) branch, not the v2.x branch! This last one is important. Regular fixes and updates should (first) happen in the 1.x branch, and only commits with API changes should be in the v2.x branch, otherwise the two branches will diverge too much, slowing development in the long run.

To easy things, I've tagged all issues with API changes, and also added a 2.0 milestone.

Some items that affect lots of smaller parts n the code, like documentation changes and fixing of trailing whitespace are tedious with multiple branches, so I recommend to make these type of changes either now or wait till 2.0 is released.

Note that this is also the time to propose code restructuring. I personally think it is in a very good shape, but here are two suggestions in case someone likes to pick it up: clean-up of function names (e.g. a region.get_chunks does not return a chunk or nbt tag, but only the chunk coordinates; most of the function names in chunk.Chunk are still specific for block ids, without data ids.). Some changes to help speed things up (either by changing some API function so faster numpy functions can be used, or addition of caches).

Collaborator

macfreek commented Apr 19, 2012

@stumpylog I tried to do some cherry-picking to ensure that the NBT code would not diverge too much from your code, as you seem willing to contribute. However, it might be easier if you would just send some pull requests, especially with code changes that are all over the place.

Would it be possible to move your changes in examples/map.py to a different branch and do make a pull request with all documentation and whitespace changes?

Something along the lines like:

git branch documentation twoolie/master
git checkout documentation
git cherry-pick -n -m 2 stumpylog/master
git status
git checkout twoolie/master examples/map.py
git status
git commit -C 425730ca7304bb6f0c9596efb20ccd01299cf040
git pull origin
(make pull request)

If you like I can do the above as well :), in any case you may want to rebase your changes to examples.py and/or your two new branches. To be honest, I don't know of a recommended way to deal with this situation -- as you can see from my experimental branch, I had a whole slew of changes, but only after some time created new branches which cherry-picked the sane commits, and pulling those one-by-one.
Explanation: create new branch called "documentation", make it the active branch, fill it with everything from your branch (the -m is required because your doing a cherry pick of a merge commit, and git needs to know which parent to use. It is either -m 1 or -m 2). Check if the status is correct (if it is empty, try again with the other -m option). It should include changes in nbt and examples. Purge the example change by checking out the version in the upstream branch. Finally commit using the log from your previous commits.

Collaborator

macfreek commented Apr 19, 2012

@stumpylog I just tried the above, but it seems your branch still does not have all fixes (e.g. the docstring of BlockArray.getblock()). Hence, I just did a cherry-pick of your 425730c. While I'm not a big fan of rebasing, in this case, it may be useful to rebase your Numpy and ChunkAPI branches. Before you do that, you may want to add a branch to your map.py changes, to make sure those commits are preserved.

git branch map c7232f3b4632d84ee451e141863839eb10f6eac9
Contributor

stumpylog commented Apr 19, 2012

@macfreek. I've created the documentation branch as you described above. Since you cherry-picked, should I still create a pull request, or is that missing a few of the commits?

On a related note, what is the best practice for making changes and branches? Should all changes for say, documentation, go into a separate branch just for documentation changes? I don't want to make more work for the people who know what they are doing.

On Apr 19, 2012, at 10:37 PM, Freek Dijkstrareply@reply.github.com wrote:

@stumpylog I just tried the above, but it seems your branch still does not have all fixes (e.g. the docstring of BlockArray.getblock()). Hence, I just did a cherry-pick of your 425730c. While I'm not a big fan of rebasing, in this case, it may be useful to rebase your Numpy and ChunkAPI branches. Before you do that, you may want to add a branch to your map.py changes, to make sure those commits are preserved.

git branch map c7232f3


Reply to this email directly or view it on GitHub:
#55 (comment)

Collaborator

macfreek commented Apr 20, 2012

The pull request is not needed with the cherry-picking (it seems all changes left are some whitespace changes).

I recommend to have one master branch which closely follows the upstream. If you are working on multiple features, it may be useful to have multiple branches, so your pull requests are more atomic, but in general that's only needed if you have a pending pull requests, and you want to continue coding while the upstream has not accepted your pull request. (if you would continue coding in the same branch, your new commits are appended in the existing pull requests).

I recommend to keep the number of branches limited. Documentations should normally not have it's own branch, but just be pulled quickly. The only reason I suggested it in this case is because your master still has the map.py and I don't think that's ready yet. Hence I could not simply merge your master branch. If you would move those map changes out of your master, and make sure your master is close to the upstream (twoolie/master), that would certainly make it easier for you and I to merge and make pull requests.

As for people who know what they are doing: I'm still learning ;) You should mostly do what works for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment