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

Removed unnecessary space in application.rb #1315

Merged
merged 3 commits into from May 26, 2011
Merged

Conversation

ardavis
Copy link

@ardavis ardavis commented May 25, 2011

I removed the extra leading space in the application.rb that is generated with 'rails new'.

@ardavis
Copy link
Author

ardavis commented May 25, 2011

To clarify why I removed the extra space in application.rb and added it to the comment_if helper is because when the following four lines are generated:

require "active_record/railtie"
require "action_controller/railtie"
require "action_mailer/railtie"
require "active_resource/railtie"

The first one had a leading space, causing it to be not formatted correctly.

josevalim added a commit that referenced this pull request May 26, 2011
Removed unnecessary space in application.rb
@josevalim josevalim merged commit badc72f into rails:master May 26, 2011
@sikachu
Copy link
Member

sikachu commented May 27, 2011

Somehow I wish those three commits to be rebased together before get merged. This is too easy.

@ardavis
Copy link
Author

ardavis commented May 27, 2011

How would one do that? I'm extremely new to rails, and programming altogether actually. It seems to be too late now, since it's already merged, but for future reference?

Thanks!
Andy

@sikachu
Copy link
Member

sikachu commented May 27, 2011

Aha, I'll explain to you for your further reference. Your keywords for today are git squash commit and git force push ;)

First you'll want to look up for the commit before your commits. you can do so by git log. You'll see something like this:

commit 60ca8d95938eddd2e9a54fb1167fd636e510b914
Author: Andrew Davis <Lordsauron90@gmail.com>
Date:   Wed May 25 19:18:29 2011 -0400

    Added a space to the comment_if helper for formatting purposes

commit 063229525f8a89cd3dd6efa4c0cead37e42d2bd6
Author: Andrew Davis <Lordsauron90@gmail.com>
Date:   Wed May 25 19:04:15 2011 -0400

    Accidentally added extra IDE files, removed them.

commit 3b23759a74009cc76237f60de1cd60d0d0c97d34
Author: Andrew Davis <Lordsauron90@gmail.com>
Date:   Wed May 25 19:03:29 2011 -0400

    Removed unnecessary space in application.rb for formatting

commit 9b6791870bbe67f5c93cad0a676148b7cc83e697
Merge: 7fd0a6d 9cafc28
Author: José Valim <jose.valim@gmail.com>
Date:   Wed May 25 14:55:04 2011 -0700

    Merge pull request #1312 from joshk/remove_active_support_deprecations

    Removed deprecated methods and related tests from ActiveSupport

So now we see the SHA of that commit, we can then run "interactive rebase" with that.

git rebase -i 9b6791870bbe67f5c93cad0a676148b7cc83e697

This will open your text editor with the list of commits. It will be something along the line like this:

pick 3b23759 Removed unnecessary space in application.rb for formatting
pick 0632295 Accidentally added extra IDE files, removed them.
pick 60ca8d9 Added a space to the comment_if helper for formatting purposes

# Rebase 9b67918..60ca8d9 onto 9b67918
#
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message
#  e, edit = use commit, but stop for amending
#  s, squash = use commit, but meld into previous commit
#  f, fixup = like "squash", but discard this commit's log message
#  x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#

If you look at the comments there, you'll see that we need to change pick to squash. Your result will look like this:

pick 3b23759 Removed unnecessary space in application.rb for formatting
squash 0632295 Accidentally added extra IDE files, removed them.
squash 60ca8d9 Added a space to the comment_if helper for formatting purposes

Now if you save and quit that file, another editor will come up and asking you to change commit message.

# This is a combination of 3 commits.
# The first commit's message is:
Removed unnecessary space in application.rb for formatting

# This is the 2nd commit message:

Accidentally added extra IDE files, removed them.

# This is the 3rd commit message:

Added a space to the comment_if helper for formatting purposes

Then you just change your wording to describe those commits. After that, save and close your editor. It should say something like this:

Successfully rebased and updated refs/heads/master.

Now you can force push to your remote. I assume that you have a remote name ardavis

git push ardavis +master

You'd then see that it pushes successfully and it would also say "forced update" in that log. Now wait for about 1 minute and your pull request will automatically updated with new commit.

Note Note Note

Always use git rebase and force push only if you're the only single developer on that repository. This will be fine with your fork of Rails, as you're the single person accessing/pushing it. I would not recommend you do something like this if you're sharing a repository with somebody else, as they would facing an error that the original commit was disappeared and now they won't be able to run git pull anymore.

@ardavis
Copy link
Author

ardavis commented May 27, 2011

Thanks! I'll try to do that later. I appreciate it!

@josevalim
Copy link
Contributor

I agree, I will be more careful next time. The problem is that I usually don't look at the number of commits, just the diff after the pull request. And the diff showed just one line change. :(

@ardavis
Copy link
Author

ardavis commented May 27, 2011

I followed your excellent instructions sikachu, I forced push to my fork. So I guess that's all I have to do then?

Sorry José for the mixup.

Thanks guys, definitely a learning process :)

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

4 participants