Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename class for consistency #15

Closed

Conversation

achambers
Copy link
Contributor

Just a quick redefine of the District class to be consistent with the way the rest are defined

@achambers
Copy link
Contributor Author

Hmm...first Pull Request. Not sure if I did it right. Didn't mean for the .gitignore to be in there. Oh well. Can't hurt. Any probs, let me know.

end
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add a newline here, please?

@steveklabnik
Copy link
Owner

Thanks so much! You did good!

So, here's what I need from you:

  1. Add that newline to the end of the file, commit that.
  2. We need to get rid of that merge commit. You can do that by doing this:
$ git rebase -i master

This will bring up a screen that looks something like this:

pick c4cf69c Updated .gitignore
pick dabc765 Merge remote-tracking branch 'upstream/master'
pick ff5e9b5 Modified class definition for consistency

What you need to do is delete the second line, so that it looks like this:

pick c4cf69c Updated .gitignore
pick ff5e9b5 Modified class definition for consistency

When you delete that line, it tells git to throw away that commit. You don't need it any more; the rebase will update things properly.

Once that's done, go ahead and push the branch up to GitHub. Since you've changed history, you'll need to force push:

$ git push origin rename-class-for-consistency -f

Normally, you don't want to force push, as it could mean that you're messing stuff up. The -f is for 'force.' In this case, you know what you're doing, so forcing it is the right thing to do.

Then, GitHub automatically updates the pull request! Tell me if you have any problems with that, and when you're done. Thanks so much!

@achambers
Copy link
Contributor Author

Okie doke. All makes sense. Will make the changes tomorrow morning. Thanks
again for your help

On Sat, Apr 13, 2013 at 12:25 AM, Steve Klabnik
<notifications@github.com<javascript:_e({}, 'cvml',
'notifications@github.com');>

wrote:

Thanks so much! You did good!

So, here's what I need from you:

  1. Add that newline to the end of the file, commit that.
  2. We need to get rid of that merge commit. You can do that by doing
    this:

$ git rebase -i master

This will bring up a screen that looks something like this:

pick c4cf69c Updated .gitignore
pick dabc765 Merge remote-tracking branch 'upstream/master'
pick ff5e9b5 Modified class definition for consistency

What you need to do is delete the second line, so that it looks like this:

pick c4cf69c Updated .gitignore
pick ff5e9b5 Modified class definition for consistency

When you delete that line, it tells git to throw away that commit. You
don't need it any more; the rebase will update things properly.

Once that's done, go ahead and push the branch up to GitHub. Since you've
changed history, you'll need to force push:

$ git push origin rename-class-for-consistency -f

Then, GitHub automatically updates the pull request! Tell me if you have
any problems with that, and when you're done. Thanks so much!


Reply to this email directly or view it on GitHubhttps://github.com//pull/15#issuecomment-16322695
.

@achambers
Copy link
Contributor Author

Hmm....Not sure if I successfully got rid of the merge commit. When doing the 'rebase -i master' it never showed me the line:

pick dabc765 Merge remote-tracking branch 'upstream/master'

@steveklabnik
Copy link
Owner

It's still there, but don't worry about it. I can fix it myself when I merge it in. (You can tell because the commits tab on this request still shows it)

Ill try to do tha tonight or tomorrow, I'm running a RailsGirls today.

@steveklabnik
Copy link
Owner

Thank you! ❤️

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.

None yet

2 participants