search

xofbd / wqu-weather-app

A demo web application for WorldQuant University Introduction to Data Science Module
GNU General Public License v3.0
7 stars 11 forks source link

Acknowledge contribution by gammaG2 #8

Open xofbd opened 4 years ago

xofbd commented 4 years ago

Looks like when I merged the PR, I must have kept the default which squashed the commit, thus history doesn't clearly show @gammaG2 contribution. Further, I've added a contributors section to the README. @gammaG2 any chance you want to create a new PR where you update the README? Also, I'll make sure to use the correct merging option to preserve your commit.

gammaG2 commented 4 years ago

Thanks for the thought. I shall do the same in my next PR.

I would like to understand what you meant by

I must have kept the default which squashed the commit, thus history doesn't clearly show

More clearly I want to understand how such an issue happens in git(did the commit get overwritten or reset) and how to prevent the same in our future projects.

xofbd commented 4 years ago

More clearly I want to understand how such an issue happens in git(did the commit get overwritten or reset) and how to prevent the same in our future projects.

so what I think happened is that GitHub offers several options for merging in PR. One option (which may be the default) is to squash all commits from the branch into one commit. The author of the squashing is whoever merged the branch into master. And that was me.

I will be more careful this time. In fact, I should create a practice branch (e.g, staging or fake-master) and merge your new PR to that branch. If I figure out the best merging option, then we'll use that option. Makes sense?

gammaG2 commented 4 years ago

I understand a bit what you are saying, but for clarity sake, could you please tell which is better? Like for e.g my PR had 4 commits to my branch, if you squash and merge, then the net change i did from 4 commits is merged as 1 commit to origin master. Isn't this preferable as it keeps the commit history cleaner? Or is the squashing equivalent to working around the very basic concept of distributed VCS?

What's more important and acceptable for any repo maintainer? keeping entire version history as is , or keeping it clean?

xofbd commented 4 years ago

You bring up a source of debate about the preferred method. I personally don't like the automatic squashing of commits. Some changes to merge in are best committed to history as several commits. Of course, if someone is looking to merge a bunch of commits, I will suggest they rebase, e.g., rebase -i HEAD~n and clean up their branches history. I am leaning towards the rebase merge, which is what we use at The Data Incubator.

See https://docs.github.com/en/github/administering-a-repository/about-merge-methods-on-github

Let me know what you think.

gammaG2 commented 4 years ago

Thank you! I have a much clearer idea of how the different merges work and the differences between them.

My takeaway is that, in absence of a mandated merge policy by the project team, it is up to the repo manager to decide which method to use, keeping in mind how many commits are there and how much actual change is there amongst the single commits.

Would that be a fair statement?

xofbd commented 4 years ago

Yeah that's a fair take-away. I Ideally, these details would be in the contributing policy of the repo so contributors know the system. For full disclosure, I've mostly been on the other end, so it's a learning experience for me with regards to managing a repo.

gammaG2 commented 4 years ago

Original issue is closed by merging #10 I believe.