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

Add the ability to specify body from command-line #183

Closed
wants to merge 1 commit into from

Conversation

vhata
Copy link

@vhata vhata commented May 18, 2012

There is no way to specify a body for a pull-request using hub unless
you omit the title and let hub open an editor for you.

This commit adds the -B parameter to specify a body (and adds -t for
title for consistency's sake).

There is no way to specify a body for a pull-request using hub unless
you omit the title and let hub open an editor for you.

This commit adds the -B parameter to specify a body (and adds -t for
title for consistency's sake).
@danielribeiro
Copy link

+1

@mislav
Copy link
Owner

mislav commented Nov 16, 2012

Hey @vhata, sorry this didn't get my attention for such a long time. Generally I'm in favor of being to specify all things via command-line. I would like to have ability to pull this information from a file or STDIN, too. I'll pull your code and work from there the next time I work on hub.

You did a good job updating the documentation but you forgot tests.

@kyanny
Copy link

kyanny commented May 10, 2013

@mislav

I added two commits to @vhata's commit.

I didn't make another pr because I want to respect @vhata's original commit.
You can apply my commits via https://github.com/kyanny/hub/commit/36a4eda0445c6f158bead386a1a0ad41f087a601.patch and https://github.com/kyanny/hub/commit/e290567da108e192e6c479aaa78100e186201f73.patch.

How about it?

@mislav
Copy link
Owner

mislav commented May 10, 2013

You could have opened another pull request which includes vhata's commit plus your own. But it's OK this way as well; it's not hard for me to pull all this. But I stopped maintaining unit tests in test/ directory and instead started porting everything to Cucumber in features/ directory. It would be great if you found the time to do the same with the couple of tests you added/edited here in your commits.

I'm still not sure about the -B flag naming. I would rather name it --body. I'm also thinking whether title and body separation is necessary. Why not just take a single argument and extract the 1st line of it as "title" and the rest as "body"? I would also like that there's ability to take in the contents of a file or stdin.

How about emulating the interface of git commit?

  • hub pull-request -F <file> - reads title & body from a file
  • hub pull-request -F - - from stdin
  • hub pull-request -m "my text" - from argument

@mislav
Copy link
Owner

mislav commented May 11, 2013

See just referenced commit for my implementation of -F and -m support. I plan to release this. Please comment if you have any thoughts.

Basically, instead of having to pass in 2 arguments (title & body) you just pass one big string (either via argument, stdin or file) where the first block of text is the title and everything else is body.

kyanny added a commit to kyanny/hub that referenced this pull request May 11, 2013
This ability is same as mislav#183 (made by @vhata) and mislav#256 (made by @banyan).
The name of `-m` option imitates `git commit`.

Add test (cucumber feature), too.
@kyanny
Copy link

kyanny commented May 11, 2013

@mislav

Thank you for your fast work!
I just did almost same thing at https://github.com/kyanny/hub/compare/pr-with-body but your's looks better.

Just my two cents, I think the read_msg method is not necessary because read_editmsg can handle both STDIN and FILE with tiny fixes.

@mislav mislav closed this in 37accd4 May 12, 2013
@mislav
Copy link
Owner

mislav commented May 12, 2013

Landed in master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants