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

Clarify the fixup instructions for the precommit hook. #22

Merged
merged 1 commit into from
Oct 27, 2015

Conversation

alanf
Copy link
Contributor

@alanf alanf commented Oct 27, 2015

Let the people know that there's a fast way to fix and stage changes!

screen shot 2015-10-27 at 12 25 48 pm

@alanf
Copy link
Contributor Author

alanf commented Oct 27, 2015

cc @orta

# This is what the dev can run to fixup an individual file
echo "\"$DIR\"/format-objc-file.sh '$file' && git add '$file';"
echo -e "\"$DIR\"/format-objc-file.sh '$file' && git add '$file';"
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: I don't think you need the -e here, because there is no \n on the line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

With the below "fix all" message, it seems reasonable to remove the git add here, which would also address @orta's issue.

@kyleve
Copy link

kyleve commented Oct 27, 2015

LGTM

@justinseanmartin
Copy link
Collaborator

LGTM as well.

@alanf alanf force-pushed the alanf/clearer-instructions branch from 99c9061 to 14d726e Compare October 27, 2015 20:21
alanf added a commit that referenced this pull request Oct 27, 2015
Clarify the fixup instructions for the precommit hook.
@alanf alanf merged commit 5c18fde into master Oct 27, 2015
@orta
Copy link

orta commented Oct 28, 2015

Ace!

@dfed dfed deleted the alanf/clearer-instructions branch October 28, 2015 18:05
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