Skip to content

Documentation Improvement#18675

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
sturkmen72:update-documentation
Nov 18, 2020
Merged

Documentation Improvement#18675
opencv-pushbot merged 1 commit intoopencv:3.4from
sturkmen72:update-documentation

Conversation

@sturkmen72
Copy link
Copy Markdown
Contributor

@sturkmen72 sturkmen72 commented Oct 28, 2020 •
edited
Loading

documentation improvement

ready to review

force_builders_only=docs

@sturkmen72 sturkmen72 changed the base branch from master to 3.4 October 28, 2020 01:38
@sturkmen72 sturkmen72 changed the base branch from 3.4 to master October 28, 2020 01:39
@sturkmen72 sturkmen72 changed the base branch from master to 3.4 October 28, 2020 01:52
@sturkmen72 sturkmen72 force-pushed the update-documentation branch from 981c48c to 7fb624b Compare October 28, 2020 01:53
@sturkmen72 sturkmen72 changed the title Update imgcodecs.hpp Documentation Improvement Oct 28, 2020
@sturkmen72 sturkmen72 force-pushed the update-documentation branch from 7fb624b to 5583c0f Compare October 28, 2020 02:03
@asmorkalov asmorkalov changed the title Documentation Improvement WIP: Documentation Improvement Nov 12, 2020
@sturkmen72 sturkmen72 force-pushed the update-documentation branch 2 times, most recently from 5c60141 to e8a4800 Compare November 17, 2020 21:32
@sturkmen72
Copy link
Copy Markdown
Contributor Author

sturkmen72 commented Nov 17, 2020

@Tetragramm could you check sentence changes in this PR

Tetragramm
Tetragramm reviewed Nov 17, 2020
View reviewed changes
modules/highgui/include/opencv2/highgui.hpp Outdated

/** @brief Selects ROIs on the given image.
Function creates a window and allows user to select a ROIs using mouse.
The function creates a window and allows the user to select a ROIs using mouse.
Copy link
Copy Markdown
Contributor

@Tetragramm Tetragramm Nov 17, 2020

Choose a reason for hiding this comment

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

Would change to "...select multiple ROIs..."

@Tetragramm
Copy link
Copy Markdown
Contributor

Tetragramm commented Nov 17, 2020

Everything else looks good.
If we want to be extremely pedantic, we can do "using the mouse." on the two selectROI functions, but it's perfectly understandable as is.

@sturkmen72
Copy link
Copy Markdown
Contributor Author

sturkmen72 commented Nov 17, 2020

@Tetragramm Thank you very much.

@sturkmen72 sturkmen72 changed the title WIP: Documentation Improvement Documentation Improvement Nov 18, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 18, 2020

@sturkmen72 Is the patch ready for review?

@sturkmen72
Copy link
Copy Markdown
Contributor Author

sturkmen72 commented Nov 18, 2020

@asmorkalov yes, the patch is ready for review.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 18, 2020

Looks good to me 👍
IMHO, the mouse and the user are redundant. Just mouse and user sounds better.

@asmorkalov asmorkalov self-requested a review November 18, 2020 11:41
@asmorkalov asmorkalov self-assigned this Nov 18, 2020
@sturkmen72
Copy link
Copy Markdown
Contributor Author

sturkmen72 commented Nov 18, 2020

@asmorkalov before correcting white space problem let me know if adding link is good or not.
preview

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 18, 2020

https://www.learnopencv.com/how-to-select-a-bounding-box-roi-in-opencv-cpp-python/ is external resource. I prefer not not add external links besides white papers, external projects, standards, etc.

@sturkmen72
Copy link
Copy Markdown
Contributor Author

sturkmen72 commented Nov 18, 2020

@asmorkalov thank you for reviewing.

The function creates a window and allows users to select a ROI using the mouse.
The function creates a window and allows users to select multiple ROIs using the mouse.

i hope this sounds better.

@opencv-pushbot opencv-pushbot merged commit 328883b into opencv:3.4 Nov 18, 2020
@sturkmen72 sturkmen72 deleted the update-documentation branch November 18, 2020 17:33
This was referenced Nov 20, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

+1 more reviewer

@Tetragramm Tetragramm Tetragramm left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@asmorkalov asmorkalov

Labels

category: documentation Documentation fix or update

Projects

None yet

Milestone

3.4.13

Development

Successfully merging this pull request may close these issues.

5 participants

@sturkmen72 @Tetragramm @asmorkalov @opencv-pushbot @alalek