Skip to content

Conversation

narimiran
Copy link
Member

No description provided.

Also, change some of `echo`s to `doAssert`.
Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

@Araq Araq merged commit f0be575 into nim-lang:devel Jan 23, 2019
@timotheecour
Copy link
Member

timotheecour commented Jan 23, 2019

/cc @dom96 @Araq @narimiran

to anyone interested in making review for stuff like that easier:

github doesn't help at all with code block moves, and it's a bit tricky to find the right cmd line to see the diff properly to see what changed/deleted vs what moved.
Here's an option that works great here, preventing reviewer from missing anything:

git fetch origin pull/10431/head && git checkout FETCH_HEAD # checks out in detached mode to avoid polluting tree (you should create a bash function for that)
git show --color-moved-ws=allow-indentation-change --color-moved=blocks HEAD

(for HEAD: 758b358)

Note: I also have this in my ~/.gitconfig, which highlights github-like changes within a block to see what changed vs didn't change (that's an orthogonal concept to what was moved)

[core]
  pager = "diff-so-fancy | less -R"

here's the diff (snippet): it highlights properly what was changed/deleted/re-indented

image
...

this snippet shows large code moves that can be (mostly) ignored:

image

@Araq
Copy link
Member

Araq commented Jan 23, 2019

I'm gonna try this, thanks!

@narimiran narimiran deleted the fix-9671 branch January 30, 2019 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants