Add ft/sec conversion

[esclear]

Add test along with it

Closes #55

[nalinbhardwaj]

@cannawen squashed and merged. Checkout the commit message and it's format by the way, I really like that style much more(as I mentioned in #50)

[cannawen]

@nalinbhardwaj Oh cool, I didn't know a squash-merge looked like that!

Can you open a new issue to discuss all the different kinds of merging (Merge vs. Squash Merge vs. Rebase Merge)?

I feel like different merges might makes sense in different contexts (i.e. large changes with well-defined commits -> regular merge, small change with many commits -> squash merge) but I'm not 100% sure what each merge does right now so I will chime in with my opinion once I figure out how exactly they behave :P

[nalinbhardwaj]

@cannawen IMO, squash merge is the way to go for all kinds of changes(small or large), since it says the PR number right there, one can use the PR diff tool to trace back to exact commit where it changed inside the branch. I don’t see a downside to squash-merges tbh, only upsides. Also, this would prevent polluting commit history with back traces in time too, as that makes it very confusing in merge conflicts. I don’t think it needs a seperate issue. If you feel the need to discuss this let me know.

[cannawen]

@nalinbhardwaj What do you mean back traces in time? Do you mean like the pink line here, that was a based few commits behind master when it was merged? If so, how does that affect merge conflicts?

I find value in squash-merging some PRs but I disagree that squash-merge is always the answer. My arguments against squash-merging 100% of the time:

1) It is sometimes useful to see individual commits 2) It is sometimes useful to see when a PR was branched off of master 3) "Dangling" branches that are left open are confusing

Further elaboration:

It is sometimes useful to see individual commits.

If we squash merge, it would be impossible to see individual commits without using GitHub PR history (which I find cumbersome - I like SourceTree!)

When I make a big change, I like making small commits for each part of the code I finish. If I finish writing a function, I might make a commit that describes what the function does. This way, in the future when I git blame the code to figure out why a change is made, I can see a clear commit message describing exactly what is happening.

This is especially helpful for non-obvious changes (like a change in X module is needed so module Y would work).

If we squashed down all commits, any work-in-progress chunks of work would be lost, and a future contributor would just see a giant commit, which would be harder to understand. (X, Y, Z, A, B are changed but there is no obvious connection between X and Y)

I also noticed that some of our contributors TDD'd their code, and committed their failing tests first. This is not really "useful" to know, but cool to see their process in our history!

It is sometimes useful to see when a PR was branched off of master

Real-life development does not happen in a straight line. Squash-merging may give a misleading view of history. Looking at the my lovely test repo, commits 13 and 12 (in PR number 5) have never been tested with commit 11 or 10, but the git tree does not represent that fact.

If a PR merges in "cleanly" without conflicts, it can still breaks the app due to a change in master upstream. If we squash-merge, we lose the point at which the branch was developed from. This is especially problematic if the bug is not noticed until much later on, and it's harder to track down.

If we regular-merge, we can see which commits from upstream of the dev branch could have caused the bug to occur. Again, I'm not sure if someone would be able to track down the starting point of the original branch through the PR? But, looking at your local git repository, I don't think it would be possible.

"Dangling" branches that are left open are confusing

Test repo again: I find it confusing when branches are merged, but still exist on origin. All of the branches above are merged, but it only "obvious" looking at the tree that the regular-merge branch is merged.

This is the worst when I have old branches but I don't know if they are merged, I have to go hunt for the same commit messages in master or manually check the code in master to see if it's there ... #triggered

Good branch discipline of always deleting merged branches will help with this. We should be deleting branches even if we do a regular merge, but if we forget, it is easier to tell the branch is already merged into master and safe to delete.

---

OK, so I've written a lot about when I think squash-merge is not always the answer. But I don't like a uselessly ugly commit tree (I think some of the "ugliness" of the branches is useful), so I do think squash-merge has a place.

When I think it's appropriate to squash-merge: When a contributor unnecessarily makes a lot of commits

When I think it's appropriate to rebase-merge: Never. Notice how the option is disabled? evil laughter

When I think it's appropriate to regular merge: Every other time

And... that's all I've thought of so far. Maybe more thoughts will come later

[nalinbhardwaj]

@cannawen definitely agree with a lot of those thoughts, I think it's okay in that case to do a regular merge, just some random things:

If we squash merge, it would be impossible to see individual commits without using GitHub PR history (which I find cumbersome - I like SourceTree!)

I myself have always just used git CLI so I'm not very aware about this. At least for the CLI client, if you use cpanm git-pr it's extremely quick and simple to trace everything quickly if you know the PR# (Infact, I'm typing this comment from git-pr 😛)

When I make a big change, I like making small commits for each part of the code I finish. If I finish writing a function, I might make a commit that describes what the function does. This way, in the future when I git blame the code to figure out why a change is made, I can see a clear commit message describing exactly what is happening.

I myself, often make large number of commits offline, and squash them to meaningful commits later, so I understand that train of thought, but I just feel like simply merging stuff, in commit history looks extremely weird. Interspersed commits out of context, etc. But I think it's fine for PRs with small number of commits(1-2). Maybe one solution is to make it compulsory to have the PR# in commit details, for future reference on master, but this is totally your decision.

When I think it's appropriate to squash-merge: When a contributor unnecessarily makes a lot of commits

Okay, this sounds like a good idea too.

When I think it's appropriate to rebase-merge: Never. Notice how the option is disabled? evil laughter

I agree with this of course.

[cannawen]

What do you mean interspersed commits out of context? Like, if you order them by time? Wow, you know what, I've never actually looked at git commits ordered by time so that didn't even cross my mind ...

Update: I just took a look at my git log for my test project, and now I totally understand what you mean! The squash-merges makes a lot more sense looking at the history there. Darn (g)it, I think we view git history in completely different ways, and that is why we are having this disagreement, lol!

I think adding a PR number to each commit would help in the chronological view, but it may be too difficult for everyone to edit their git history & force push. Maybe we can ask people to add the issue number to each commit? Then you would have to hop to the issue to hop to the PR, but at least the regular-merged commits are "tagged" with a similar number.

[nalinbhardwaj]

Sounds good to me. 😄 I'm not that good with git tbh, that's just how I've always used it 😛 maybe it's more sensible to everyone else.

[cannawen]

People use git in different ways, and I am sure "git log" is one of the most common ways (even though SourceTree users are clearly superior) ;)

This "sorting by time is confusing + merge commits" thing is a valuable insight, thank you for this discussion :D I will add some documentation around what a commit message should look like so it's easier to see which commits are linked to each other.

And sorry for all the notifications @esclear, plz forgive

[esclear]

@cannawen Well, I muted the issue long before, but thanks for @mentioning me :smiley: