search

citusdata / pg_shard

ATTENTION: pg_shard is superseded by Citus, its more powerful replacement
https://github.com/citusdata/citus
GNU Lesser General Public License v3.0
1.06k stars 63 forks source link

Reject unknown partition types #106

Closed jasonmp85 closed 9 years ago

jasonmp85 commented 9 years ago

Surprising that this conditional didn't already have an else clause, but so it goes. This causes us to reject any partition types we don't explicitly support (the function used to silently succeed).

onderkalaci commented 9 years ago

@jasonmp85 I am also surprised to see this problem. I checked this change and realized that in one of my previous commits I broke this check.

So, the change and tests look good and I merged it.

jasonmp85 commented 9 years ago

@onderkalaci did you merge this with the "merge" button in GitHub, or were there conflicts? I ask because I see a merge from develop to here, then a merge from here back to develop. Merges aren't really directional, so only one of these commits was strictly necessary, and it results in a weird history.

onderkalaci commented 9 years ago

@jasonmp85 merge button on github was disabled. So, yes I first merged develop into this branch and pushed. Then, merge button was enabled again and I merged it here.

So, for such cases, should we fix conflicts then merge directly to the master?

jasonmp85 commented 9 years ago

Ah, I see now.

It's worthwhile to merge from develop into a feature branch if you're going to continue working on that branch and just want to have the latest changes while you continue development. I prefer git rebase -i for doing this, but that rewrite the history of the feature branch and necessitates a push --force, so it's not for everyone.

If you are already ready to merge to develop, just ensure your local copy of develop is up to date, then switch to it and merge the feature branch there, making sure to use --no-ff, so git doesn't just fast-forward develop to the HEAD of the feature branch. If you need to resolve conflicts, do that then (git mergetool can help if it's configured) and then just git push from develop.

A merge just introduces a commit with two other commits as its parent, then it moves the "pointer" for a branch to point to itself. There's not really any concept of merging A into B or B into A: it's all about whether you choose to call the resulting commit A or B when you're done.

So my point was mostly just that if you already merged develop into the feature branch, you've done the merge, it's just that you named the merge commit after the feature branch instead of naming it develop. "Merging back" to develop just introduces a second merge commit and names it develop, so that second commit is essentially vacuous.

tl;dr version: If you can't click the button and just want to merge, ensure your branches are up to date then do it locally with git merge --no-ff.