Skip to content

Fix cv::FileStorage::Mode::Memory doxygen layout#21582

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
gfiumara:3.4
Feb 9, 2022
Merged

Fix cv::FileStorage::Mode::Memory doxygen layout#21582
opencv-pushbot merged 1 commit intoopencv:3.4from
gfiumara:3.4

Conversation

@gfiumara
Copy link
Copy Markdown
Contributor

@gfiumara gfiumara commented Feb 8, 2022

Multiline post-variable doxygen comments need a terminator. Without this PR, the previous comments appear as two separate comments in the generated HTML documentation in reverse order.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=docs

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

Comment on lines +316 to +318
MEMORY = 4, //!< flag, read data from source or write data to the internal buffer (which is
//!< returned by FileStorage::release)
//!<
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe it makes sense merge these lines into one instead.

3.4 version is not broken.
4.x version is broken.

Patch "live" changes: enum cv::FileStorage::Mode

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to make it single-line, however this creates a 139 character long line that appears to go against the coding style guide. Please advise if this is an allowed exception.

No issue changing the target branch if necessary. That should be a clean patch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is OK to exceed lines length with comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've made this correction. Marking comment as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please revert target branch change and target to 3.4?

There is similar problem on 3.4 branch. But due to some reason it doesn't trigger buggy doxygen behavior.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BTW, It makes sense to put multi-line doc string as multi-line C++ comment:

        MEMORY      = 4, /**< flag, read data from source or write data to the internal buffer
                              (which is returned by FileStorage::release) */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switch back to 3.4. Made this multiline C++ comment, retaining the original line break location.

@gfiumara
Copy link
Copy Markdown
Contributor Author

gfiumara commented Feb 9, 2022

Retargeted 4.x branch with force push and PR edit.

@gfiumara gfiumara changed the base branch from 4.x to 3.4 February 9, 2022 17:24
@gfiumara
Copy link
Copy Markdown
Contributor Author

gfiumara commented Feb 9, 2022

Re-targeting 3.4, per assigned milestone.

@opencv-pushbot opencv-pushbot merged commit 9603b68 into opencv:3.4 Feb 9, 2022
@alalek alalek mentioned this pull request Feb 10, 2022
@alalek alalek mentioned this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug category: documentation Documentation fix or update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants