Skip to content

Rename cursor_path to cursor - #54

Closed
stokhos wants to merge 2 commits into
kneasle:masterfrom
stokhos:rename_cursor_path
Closed

stokhos wants to merge 2 commits into
kneasle:masterfrom
stokhos:rename_cursor_path

Conversation

@stokhos

@stokhos stokhos commented Dec 20, 2020 •

Copy link
Copy Markdown
Contributor

Changed cursor_path::CursorPath to cursor::Path.
Closes #45

@kneasle

kneasle commented Dec 20, 2020 •

Copy link
Copy Markdown
Owner

It looks like these changes are correct, but this branch contains the commits from #50 as well as the renaming changes, which makes the diffs really difficult to read. Once this is fixed, I'll be able to see exactly what this PR is doing and then review it.

The way to fix this is to 'rebase' this branch onto the new upstream/master branch. If you fancy some more complex rebasing, then squashing the last two commits (c1527b7 and 8efc0ea) into one commit would be useful (but this is nice to have, not essential).

Rebasing is done differently depending on how you're using Git. I use the command line, and these commands would perform the rebase (without combining any commits):

# Make sure that your master branch is up to date with my master branch
git checkout master
git fetch upstream
git pull upstream master
# Rebase this branch onto master
# (this will essentially 'play back' your changes but on top of the new master branch)
git checkout rename_cursor_path
git rebase master

What should happen is that the first 4 commits (which are from #50) should disappear and you'll be left with just the last 2. You'll then have to force-update your public branch on GitHub with

git push origin --force-with-lease

(supplying your username and password).

I can do the rebase if necessary, but it's a useful thing to learn.

@stokhos

stokhos commented Dec 20, 2020

Copy link
Copy Markdown
Contributor Author

I will fix this.

@kneasle

kneasle commented Dec 20, 2020

Copy link
Copy Markdown
Owner

Ayy this is much better!

Do you want to do an 'interactive' rebase to combine the commits? It would make a lot more sense in the commit history if the refactoring was done in one step.

I can provide CLI instructions if you want them, but it involves quite a lot of steps so if you're using git a different way then there are probably instructions for that.

@stokhos

stokhos commented Dec 20, 2020 •

Copy link
Copy Markdown
Contributor Author

I'd like to try it. I personally don't use git very often.

@kneasle

kneasle commented Dec 20, 2020

Copy link
Copy Markdown
Owner

Cool - it's very satisfying to be able to rewrite history and make mistakes disappear 😁. Also, I'm heading off to bed now - I'll respond to this in the morning.

@kneasle

kneasle commented Dec 21, 2020 •

Copy link
Copy Markdown
Owner

Oh yeah also, GitHub needs the word fixes or closes directly next to the issue number for it to link the issue to this PR.

So fixes issue #45 won't work, but fixes #45 will (which is dumb, I know). I've edited your comment to do that - hope you don't mind :).

@stokhos stokhos closed this Dec 22, 2020
@stokhos
stokhos deleted the rename_cursor_path branch December 22, 2020 16:43
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.

Rename CursorPath to simply Path

2 participants