search

invana / invana-studio

Open source graph visualiser.
Apache License 2.0
174 stars 19 forks source link

code readability fixes in LightenDarkenColor function #60

Closed rrmerugu closed 4 years ago

rrmerugu commented 4 years ago

fixes #57

rrmerugu commented 4 years ago

Thanks @mbrukman for the detailed explanation and pointers. I understood what you mean after experimenting with the code here https://codesandbox.io/s/lighten-darken-color-experiments-6p0pt?file=/index.html:584-608.

I have updated the code with your inputs. Do you think I can merge this now !

rrmerugu commented 4 years ago

Alrighty :) I will squash the commits.

rrmerugu commented 4 years ago

OMG!, I think I really messed up squashing the commits part. I have to live with this for now I guess. I will just merge for now :( . Really appreciate the inputs @mbrukman . Thanks a ton.

mbrukman commented 4 years ago

@rrmerugu — you can also enable click-to-squash-and-merge on GitHub settings if that's easier than the git CLI approach.

Also, for a clean history, rather than merging branches when developing, I typically sync and rebase, e.g., in this case:

# Update local copy of `develop`
$ git checkout develop
$ git pull origin develop --ff-only

# Rebase `issue-57` on `develop` and push to the PR
$ git checkout issue-57
$ git rebase develop
$ git push -f

Also, to avoid push -f affecting your develop branch, you can also add branch protection in GitHub settings to prevent anyone from accidentally running git push -f develop.

Hope this helps!

rrmerugu commented 4 years ago

I appreciate the notes @mbrukman . Thanks :)

rrmerugu commented 4 years ago

I just did my first squash commits and merge https://github.com/invanalabs/graph-explorer/commit/8e825297acebdbd4fe8390ebb757695d5e9b24c9 , all thanks to your pointers @mbrukman . I did this using the GitHub UI for now, but definitely looking forward for CLI in the near future. Never knew that Github interface has more than just merge.

From my understanding

  1. a typical merge will add one more merge commit along with the commits from the HEAD branch.
  2. squash and merge will combine all the commits from HEAD branch into one commit and merges that one commit on to base branch(in this case develop).
  3. rebase and merge would simply position all the commits from HEAD to the last commit of the base branch(in this case develop)

Am I understanding this correctly. ?

mbrukman commented 4 years ago

@rrmerugu — yes, I think your understanding is correct (in so far as it matches my own understanding).

Here are the GitHub docs on your options 2 and 3: