hackerspacesg / hackerspace.sg

Hackerspace.sg 2.0 using Punch
https://hackerspace.sg/
17 stars 25 forks source link

Added a guide to using the Cupcake CNC. #55

Closed rolandturner closed 10 years ago

chernjie commented 10 years ago

Please rebase your branch with the latest master :)

$ git rebase origin/master

(no need to close/re-open a pull request, just force push to your fork's master

kaihendry commented 10 years ago

@chernjie why are you asking Roland to rebase?

rolandturner commented 10 years ago

On 03/03/2014 01:21 AM, chernjie wrote:

git rebase origin/master

It was up to date at the time that I submitted the pull request, and still is:

$ git rebase origin/master Current branch master is up to date. $

What's the problem?

chernjie commented 10 years ago

The parent of your commit e0e70c3 is 95f0e5a, which is 22 commits behind 71db06d (master)

It could be that your origin is pointing to your "fork". Refer to Syncing a fork to bring your master up-to-date to this repository's master

kaihendry commented 10 years ago

@chernjie it still can be automatically merged.

chernjie commented 10 years ago

Fine, next time please make sure you compare before submitting a pull request.

chernjie commented 10 years ago

@kaihendry yes it can, that is the beauty of GIT. Rebasing is even better as it gives you nicer and more readable history.

kaihendry commented 10 years ago

So you asking Roland to make a nicer commit message? Still not sure why you were asking for Roland to rebase.

rolandturner commented 10 years ago

I find this difficult to believe (I forked, cloned, rolled my changes in, pushed and then requested pull in a matter of 2-3 minutes last night), but even if it's true, does it matter? My change set contains a bunch of new stuff in /cupcake which can't possibly conflict with anything else and a small addition to /about which should be able to merge automatically.

What is the practical impact of accepting my pull request as is?

On 03/03/2014 10:15 AM, chernjie wrote:

The parent of your commit e0e70c3 https://github.com/hackerspacesg/hackerspace.sg/commit/e0e70c3 is 95f0e5a https://github.com/hackerspacesg/hackerspace.sg/commit/95f0e5a, which is 22 commits behind 71db06d https://github.com/hackerspacesg/hackerspace.sg/commit/71db06d (master)

It could be that your origin is pointing to your "fork". Refer to Syncing a fork https://help.github.com/articles/syncing-a-fork to bring your master up-to-date to this repository's master

— Reply to this email directly or view it on GitHub https://github.com/hackerspacesg/hackerspace.sg/pull/55#issuecomment-36476665.

rolandturner commented 10 years ago

On 03/03/2014 10:24 AM, chernjie wrote:

@kaihendry https://github.com/kaihendry yes it can, that is the beauty of GIT. Rebasing is even better as it gives you nicer and more readable history.

— Reply to this email directly or view it on GitHub https://github.com/hackerspacesg/hackerspace.sg/pull/55#issuecomment-36476992.

I'm intrigued to understand your reasoning here, https://github.com/hackerspacesg/hackerspace.sg/commit/9fd50f23acbeced2f07d4a705a0edb7bc94abe30 looks perfectly readable to me. The entire point of git is that merges are only of identified changes, not of entire state. Can you explain how this would have looked nicer and/or more readable had I rebased?

chernjie commented 10 years ago

@rolandturner Don't worry, you did not do anything wrong.

The debate on whether to use rebase or merge may be a little confusing to beginners, but it has its merits. Here is short article that briefly explains the odd practice

In your particular case, yes it does not conflict with any other files since it is new. That is a given. It is, at least for seasoned git maintainers, a courtesy to future developers to make the history cleaner.

rolandturner commented 10 years ago

CJ,

I was not suggesting that I did anything wrong, but I did ask you to explain your unsupported and apparently false assertion that rebasing would have made the history clearer in this particular case. Please explain this assertion, or withdraw your request.

(I am aware of the problems with careless merging meaning that a change set gets polluted, but my change set contained only the new files plus a four line change to an existing file, all in a single commit with a clear one-line message. So far as I can tell, it's not possible to make the history any cleaner. Indeed, I performed a new clone prior to pushing and pull-requesting and moved my changes into it manually specifically to ensure that this was the case. I am not an inexperienced developer, the need to keep histories readable by others is one that I have spent years addressing with teams using far less capable systems than git. That said, I do have very little experience with github, and by the look of it misunderstood what happened when I forked hackerspace.sg last night; in particular it appears that I did not actually do so and, instead, ended up using the existing fork that I'd created during the last plenum and forgotten about; presumably I overlooked an error message at some step.)

On 03/03/2014 10:44 AM, chernjie wrote:

@rolandturner https://github.com/rolandturner Don't worry, you did not do anything wrong.

The debate on whether to use rebase or merge may be a little confusing to beginners, but it has its merits. Here is short article that briefly explains the odd practice http://blogs.atlassian.com/2013/10/git-team-workflows-merge-or-rebase/

In your particular case, yes it does not conflict with any other files since it is new. That is a given. It is, at least for seasoned git maintainers, a courtesy to future developers to make the history cleaner.

— Reply to this email directly or view it on GitHub https://github.com/hackerspacesg/hackerspace.sg/pull/55#issuecomment-36477644.

chernjie commented 10 years ago

As I have mentioned above, your master is behind the upstream/master by 22 commits. It is good practice to always bring your branch up-to-date before submitting a pull-request. That is all there is to it.

chernjie commented 10 years ago

Here is a visual guide for you on how "messy" your history looked http://screencast.com/t/zSmQATWZ Notice how the blueline stretches all the way to the bottom?

rolandturner commented 10 years ago

CJ,

In other words, it would have made no difference at all and you're wasting everybody's time. Please don't do this again.

If there is a future situation in which a better approach would have made a material difference than by all means take the time to point it out explain it (contrast, with concrete examples, how what you're proposing would have been better than what did happen), but empty appeals to good practice in situations that make no difference are pointless, covering the irrelevance of your request by describing experienced developers as beginners is offensive.

On 03/03/2014 12:02 PM, chernjie wrote:

As I have mentioned above, your |master| is behind the |upstream/master| by 22 commits. It is good practice to always bring your branch up-to-date before submitting a pull-request. That is all there is to it.

— Reply to this email directly or view it on GitHub https://github.com/hackerspacesg/hackerspace.sg/pull/55#issuecomment-36480151.

chernjie commented 10 years ago

@rolandturner You are wasting everybody's time for not making the effort to rebase. This is basic courtesy and you obviously do not have any.

rolandturner commented 10 years ago

On 03/03/2014 12:06 PM, chernjie wrote:

Here is a visual guide for you on how "messy" your history looked http://screencast.com/t/zSmQATWZ

OK, that makes a little clearer where you're coming from, but I don't understand its relevance to my pull request. My request was for several new files and one altered file to go into HEAD and it should have been assessed on that basis (i.e. you should have been looking at the content of my request, not its history).

chernjie commented 10 years ago

Yes your content is important. And as I have said, in this particular case it does not affect us too much since your file is new. But for future situation, please learn how to rebase. It will save the rest of us a lot of unnecessary headaches!

rolandturner commented 10 years ago

On 03/03/2014 12:15 PM, chernjie wrote:

@rolandturner https://github.com/rolandturner You are wasting everybody's time for not making the effort to rebase. This is basic courtesy and you obviously do not have any.

Begging your pardon CJ, rebasing has no relevance of any kind to the pull request in question. The only relevant question is what change my pull request proposes and, on that basis, it is crystal clear.

This is the second personal insult that you have made. Please consider desisting any further action until you have regained your composure.

chernjie commented 10 years ago

Firstly, apology to the "first insult". You are undeniably an experienced developer new to git, and failing to rebase is a common mistake everybody makes when starting with git (I personally blame GIT itself for introducing this problem). Perhaps I should be more careful with the "beginner" word.

It is not uncommon to request pull-requester to rebase even if the content/changeset is good. I apologize for not spending a little more time to explain in more detail.

notthetup commented 10 years ago

Although this PR was merged, the page is still 404ing.. http://hackerspace.sg/about/cupcake

rolandturner commented 10 years ago

Yes, noticed that, a difference between punch on my machine and however we're hosting it. I'll fix it when I next look at it (probably this weekend), am happy for anyone else to in the meantime.

On 03/04/2014 08:42 AM, Chinmay Pendharkar wrote:

Although this PR was merged, the page is still 404ing.. http://hackerspace.sg/about/cupcake

— Reply to this email directly or view it on GitHub https://github.com/hackerspacesg/hackerspace.sg/pull/55#issuecomment-36579649.

kaihendry commented 10 years ago

Made a quick fix http://git.io/unL6Ig via github's edit interface. Amazingly it works.

We need to complain to @laktek about how punch s behaves differently with those links. :)

rolandturner commented 10 years ago

Thank you.

laktek commented 10 years ago

Yep, I'm working on link consistency and better error reports in Punch. Should be fixed soon.

Keep those complaints coming :)