Tutorial - Make required input args positional.#15754
Merged
opencv-pushbot merged 1 commit intoopencv:3.4from Oct 23, 2019
Merged
Tutorial - Make required input args positional.#15754opencv-pushbot merged 1 commit intoopencv:3.4from
opencv-pushbot merged 1 commit intoopencv:3.4from
Conversation
Member
|
Thank you for contribution! This patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly). So, please:
Note: no needs to re-open PR, apply changes "inplace". |
Contributor
Author
|
Hi,
I made this edit on the online Github editor, without downloading/cloning the source. I changed the branch to 3.4 but I can’t do the rebase command online.
It now says that I want to merge 944 commits - is this normal?
float13 <https://github.com/float13> wants to merge 944 commits into opencv:3.4 <https://github.com/opencv/opencv/tree/3.4> from float13:patch-1 <https://github.com/float13/opencv/tree/patch-1>
There are also branch conflicts now.
Should I clone the source and then try to do the rebase? Or is it simpler to just open a new pull request directly to 3.4?
Thanks
… On Oct 22, 2019, at 6:53 AM, Alexander Alekhin ***@***.***> wrote:
Thank you for contribution!
This patch should go <https://github.com/opencv/opencv/wiki/How_to_contribute#faq> into 3.4 branch <https://github.com/opencv/opencv/wiki/Branches> first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly).
So, please:
change "base" branch of this PR: master => 3.4 (use "Edit" button near PR title)
rebase your commits from master onto 3.4 branch. For example:
git rebase -i --onto upstream/3.4 upstream/master
(check list of your commits, save and quit (Esc + "wq" + Enter)
where upstream is configured by following this GitHub guide <https://help.github.com/articles/configuring-a-remote-for-a-fork/> and fetched (git fetch upstream).
push rebased commits into source branch of your fork (with --force option)
Note: no needs to re-open PR, apply changes "inplace".
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#15754?email_source=notifications&email_token=AKLPLGFSFETCFTKSQGP57MLQP3LRPA5CNFSM4JDJPAE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB5JSWA#issuecomment-544905560>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKLPLGHHD3M7WBCWWVDYECDQP3LRPANCNFSM4JDJPAEQ>.
|
I think it would help to change all 3 of the the input file arguments to be "positional" for consistency with the other tutorials. This also simplifies the command line input to run this tutorial by reducing typing, and helpfully prints the "usage" info if any of the 3 required inputs are missing. I'm new to OpenCV and working through the tutorials. I kept getting runtime errors with this one until I realized that the arguments weren't positional, and I was missing the "--input1", "--input2, "--input3" flags preceding the filenames. All of the previous tutorials had required filenames as positional arguments and didn't require this. The original code would require each input to be specified like this: ./compareHist_Demo --input1 filename1 --input2 filename2 --input3 filename3 But with this change, the above command is simplified to: ./compareHist_Demo filename1 filename2 filename3 This avoids a confusing runtime error to make things simpler for newcomers like me :)
Member
|
I pushed rebased patch. P.S. I don't have instructions for GitHub editor about rebasing process. |
Contributor
Author
|
Thanks! Now I know for next time. Anything else to do? |
alalek
approved these changes
Oct 23, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I think it would help to change all 3 of the the input file arguments to be "positional" for consistency with the other tutorials. This also simplifies the command line input to run this tutorial by reducing typing, and helpfully prints the "usage" info if any of the 3 required inputs are missing.
I'm new to OpenCV and working through the tutorials. I kept getting runtime errors with this one until I realized that the arguments weren't positional, and I was missing the "--input1", "--input2, "--input3" flags preceding the filenames. All of the previous tutorials had required filenames as positional arguments and didn't require this.
The original code would require each input to be specified like this:
./compareHist_Demo --input1 filename1 --input2 filename2 --input3 filename3But with this change, the above command is simplified to:
./compareHist_Demo filename1 filename2 filename3This avoids a confusing runtime error to make things simpler for newcomers like me :)
This pullrequest changes
samples/cpp/tutorial_code/Histograms_Matching/compareHist_Demo.cpp