Rename top-level IO extension headers.#71
Conversation
|
Please note that the only intended changes in this PR are:
|
|
|
||
| #ifndef BOOST_GIL_EXTENSION_IO_BMP_ALL_HPP | ||
| #define BOOST_GIL_EXTENSION_IO_BMP_ALL_HPP | ||
| #ifndef boost_gil_extension_io_bmp_hpp_ |
There was a problem hiding this comment.
Is this necessary to change the style of include guards to lowercase? (The trailing underscore is not necessary either)
The Boost convention is that all symbols related to preprocessor are underscore-delimited UPPERCASE words. That has a purpose and it's a transitive knowledge across Boost codebase.
There was a problem hiding this comment.
I wasn't aware of that convention, and I have seen plenty of exceptions to that rule. Is it written somewhere, or just folklore ? (The important part is that the full filename be represented to avoid clashes. Doing my review I have seen one or two cases where a wrong guard macro was used, apparently copy&pasted from a different header. Such issues generate funny symptoms...)
So, as I had to adjust all header guards, I just used the convention I was most familiar with. Do you really think I need to change it again ?
There was a problem hiding this comment.
Is it written somewhere, or just folklore ?
Yes, it is here https://www.boost.org/development/header.html : Boost convention of all uppercase letters, with the header name prefixed by the namespace name, and suffixed with HPP, separated by underscores.
I have seen plenty of exceptions to that rule
grepping through the whole of Boost headers for ifndef\s+[a-z]+_.+ shows just one in phoenix/core/detail/index_sequence.hpp and, interestingly, handful of exceptions in Boost.Python headers, but I suspect it was a recent change :-)
Apart from the widely accepted convention, this boost_gil_extension_io_bmp_hpp_ is simply confusing with typedef-s or class names,
I can re-store upper-case, no problem, just let me know and as soon as you merge, I'll do it.
I understand the role of personal preference, but please let's
| } // namespace boost | ||
|
|
||
| #endif // BOOST_GIL_EXTENSION_IO_BMP_IO_IS_ALLOWED_HPP | ||
| #endif |
There was a problem hiding this comment.
A side comment: do we care about EOL at the EOF ?
There was a problem hiding this comment.
I think so. Why do you ask ? (I'm typically editing using emacs, and that editor automatically inserts missing EOL at the end.)
There was a problem hiding this comment.
OK. I just noticed this and (some) other headers are missing the EOL at the end.
There was a problem hiding this comment.
Hmm, emacs tells me otherwise. I just pushed a commit with the header guard fix. Didn't see any missing EOL endings.
There was a problem hiding this comment.
I'm sorry, I must be too eager trusting how GitHub renders file.
| //////////////////////////////////////////////////////////////////////////////////////// | ||
| /// \file | ||
| /// \brief | ||
| /// \author Olivier Tournaire \n |
There was a problem hiding this comment.
Side comment: Doesn't this author clash with the copyright header?
There was a problem hiding this comment.
I haven't looked at any file content.
mloskot
left a comment
mloskot
left a comment
There was a problem hiding this comment.
@stefanseefeld thanks for your effort!
Refactor IO extension headers.
2 participants
Refactor IO extension files.