search

Canadensys / narwhal-processor

Basic data processing library aiming to normalize similar values ​​in a known format.
MIT License
6 stars 7 forks source link

Belgian province/region name variations #3

Closed peterdesmet closed 11 years ago

peterdesmet commented 11 years ago

Created a dictionary with Belgian province/region name variations:

Note: The file is using empty lines to group name variations per province/region. I can remove these if necessary.

:tractor:

cgendreau commented 11 years ago

Can we squach the commits?

peterdesmet commented 11 years ago

There seem to be different opinions on the best practices for squashing commits (just Google for it):

Rebase

I use rebase on this branch: this will squash commits in the current branch, rewriting the history. In theory, you could then merge that one (or two commits).

In practice, GitHub doesn't allow rebased pushes as it is considered bad practice (= black voodoo) to do this on branches that are public. So, I should have done this before I pushed publicly, but then you wouldn't been able to see the single commits I made and if you had requested changes before merging, we would end up with more commits anyway.

Squash into another branch

This is what GitHub seems to advice. I send a pull-request, we discuss it and once everything is finished, I create a new branch (e.g. belgian-provinces-squashed where all commits are squashed. I then ask another pull request, which you accept. You close this one without merging. The disadvantage is that you have no link between the two pull requests, but you could reference them in the comment (using #).

I would suggest 2, what do you think?

cgendreau commented 11 years ago

Good, we can try the squash in another branch.

peterdesmet commented 11 years ago

:worried: I forked the narwhal-processor repository, but made my changes to the master branch (bad practice, but still learning). This means I cannot make a clear local branch based off master (as my commits are already in master).

We could make a clear branch on the canadensys/narwhal-processor, but I'm not sure how to do a merge --squash to that one (across repositories) and I don't think it's worth the effort in this case. :disappointed: Maybe next time. At least I know now never to work on master, neither on the project repository or my own. If useful, I'll update the docs to reflect this.