Skip to content

Naming some enumerations in videoio#6995

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
PkLab:i6746_naming_videoio_enums
Aug 15, 2016
Merged

Naming some enumerations in videoio#6995
opencv-pushbot merged 1 commit intoopencv:masterfrom
PkLab:i6746_naming_videoio_enums

Conversation

@PkLab
Copy link
Copy Markdown
Contributor

@PkLab PkLab commented Jul 26, 2016

resolves #6746

This pullrequest changes

This PR names base enums (API, Props and Mode Identifiers) in videoio modules mostly for documentation purposes. Enums related to specific API are left unnamed.

Enums docs are enclosed into submodules videoio_flags_base and videoio_flags_others

@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Jul 28, 2016

@mshabunin, could you please review it

@vpisarev vpisarev mentioned this pull request Aug 4, 2016
Closed
mshabunin
mshabunin reviewed Aug 4, 2016
View reviewed changes
modules/videoio/include/opencv2/videoio.hpp Outdated
2. rebuild OpenCV itself
*/
enum { CAP_ANY = 0, //!< Auto detect
enum VideoCaptureApiID {
Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Aug 4, 2016

Choose a reason for hiding this comment

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

I suggest removing ID suffix from all identifiers and use plural forms: VideoCaptureAPIs, VideoCaptureProperties, VideoCaptureModes, etc.

Copy link
Copy Markdown
Member

@alalek alalek Aug 4, 2016

Choose a reason for hiding this comment

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

and use plural forms

I believe it is useless and looks strange in general.
But it is already happened...

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 4, 2016

@PkLab , are you going to add references to these enumerations to corresponding functions documentation?

@PkLab
Copy link
Copy Markdown
Contributor Author

PkLab commented Aug 4, 2016

@mshabunin

  • Should I commit names with plurals or what else ? I think that it's useful to being explicit the that these enums are identifiers. Let me know.
  • Yes I've added references into doc in a separate branch. I'll push a PR for this as soon as enumerations names will be fixed and merged.

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 4, 2016

Let's use plural forms for consistency.

being explicit the that these enums are identifiers

Do you mean not flags? If so, I think it is not necessary, this information should be specified in the documentation block.

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 9, 2016

@PkLab , looks good. Please, squash all commits into one.

@PkLab PkLab force-pushed the i6746_naming_videoio_enums branch from a01ba21 to c33d152 Compare August 9, 2016 17:29
@PkLab
Copy link
Copy Markdown
Contributor Author

PkLab commented Aug 9, 2016 •
edited
Loading

Mistake while squashing... sorry . I'm trying to recover.
... ok "plurals" has been merge into initial commit "Naming ..."

@PkLab PkLab force-pushed the i6746_naming_videoio_enums branch from c33d152 to b303488 Compare August 9, 2016 19:06
@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 11, 2016

@PkLab , please check the following branch: master...mshabunin:pr6995
It contains single rebased commit.

@PkLab
Copy link
Copy Markdown
Contributor Author

PkLab commented Aug 11, 2016 •
edited
Loading

@mshabunin Looks good. Please could you merge your branch...is more clean than mine

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 11, 2016

@PkLab Just update this PR source branch. Something like this

git checkout -B backup_i6746_naming_videoio_enums i6746_naming_videoio_enums
git fetch https://github.com/mshabunin/opencv.git pr6995:i6746_naming_videoio_enums

# reset commiter name
git checkout i6746_naming_videoio_enums
git commit --amend # save&exit (Esc, ':wq', Enter in case of vim)

# -f is required here
git push -f origin i6746_naming_videoio_enums

@PkLab PkLab force-pushed the i6746_naming_videoio_enums branch from b303488 to 3ea0201 Compare August 11, 2016 18:36
@PkLab
Copy link
Copy Markdown
Contributor Author

PkLab commented Aug 11, 2016

@alalek done ! tank you for the tips.

Just for information:

git fetch https://github.com/mshabunin/opencv.git pr6995:i6746_naming_videoio_enums

returns
! [rejected] pr6995 -> i6746_naming_videoio_enums (non-fast-forward)
to solve non-fast-forward I've used --force

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Aug 15, 2016

👍

@opencv-pushbot opencv-pushbot merged commit 3ea0201 into opencv:master Aug 15, 2016
opencv-pushbot pushed a commit that referenced this pull request Aug 15, 2016
@mshabunin
Merge pull request #6995 from PkLab:i6746_naming_videoio_enums
b9ea59d
@PkLab PkLab deleted the i6746_naming_videoio_enums branch August 16, 2016 11:04
@PkLab PkLab mentioned this pull request Aug 24, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@mshabunin mshabunin

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Naming all enumerations in videoio.hpp

5 participants

@PkLab @vpisarev @mshabunin @alalek @opencv-pushbot