Skip to content

Update docs to be consistent with prevalent style#2491

Merged
asuessenbach merged 1 commit intoKhronosGroup:mainfrom
mikomikotaishi:main
Feb 18, 2026
Merged

Update docs to be consistent with prevalent style#2491
asuessenbach merged 1 commit intoKhronosGroup:mainfrom
mikomikotaishi:main

Conversation

@mikomikotaishi
Copy link
Copy Markdown
Contributor

@mikomikotaishi mikomikotaishi commented Feb 17, 2026

This PR addresses some of the inconsistencies seen in the docs:

  • The MySharedHandle class names its base class as base, though the rest of the library uses PascalCase, so this should be updated to Base.
  • MySharedHandle::getParent should be clearer on what it returns, so I updated the return type from const auto& to const vk::SharedInstance&. I also marked it [[nodiscard]] because it is a getter function and its value shouldn't be discarded if called.
  • Instead of commenting out the parameter name on MySharedHandle::internalDestroy, use the [[maybe_unused]] attribute to make it clear the parameter is not considered.
  • The C++ modules example used auto declarations (auto appInfo = ApplicationInfo(/* ... */);), but the rest of the library doesn't. This could instead more simply (and more briefly) be written with direct-initialisation (ApplicationInfo appInfo(/* ... */);).
  • The traditional main() signature is most often depicted as int main(), rather than with trailing return type (auto main() -> int), and there's no reason to make this arbitrary difference. Also, argv is most often depicted as char* argv[], not char* const argv[], so we might as well just stick with char* argv[] (without the const specifier).

@sharadhr
Copy link
Copy Markdown
Contributor

Thanks for trying again. I meant to fold your corrections from earlier into the big commit, but clearly I missed many. Any additional wording suggestions (in fact, I think it's a good idea to use [[maybe_unused]] for all the /* varname */ everywhere else) would be very welcome.

One note, though...

is most often depicted as char* argv[], not char* const argv[]

I think there is a safety argument here; in fact, I would const-qualify more. The safest representation of argv is char const* const argv[].

@mikomikotaishi
Copy link
Copy Markdown
Contributor Author

mikomikotaishi commented Feb 18, 2026

This is a simple example, so I don't really think the need to const-qualify as much as possible is necessary. For most people, the most recognised form of main() is int main(int argc, char* argv[]) (page 13 of the C standard doesn't const-qualify argv at all; across the rest of the document it remains un-const-qualified). I'm not against const-qualifying but seeing as it is a simple example, I think there isn't much of a reason to make it more contrived than necessary. (We might not even need to use argc and argv at all in the example as those parameters aren't even consumed in the example, but I don't mind either way.)

Also, while I didn't return to re-adding using statements, perhaps it might be worthwhile considering using statements for the modules example, as it is a full example (in a main() function) rather than a code snippet - importing symbols into scope is probably worthwhile in making the example easier to read.

@mikomikotaishi
Copy link
Copy Markdown
Contributor Author

mikomikotaishi commented Feb 18, 2026

Any additional wording suggestions (in fact, I think it's a good idea to use [[maybe_unused]] for all the /* varname */ everywhere else) would be very welcome.

There is now a PR for this, #2492.

@asuessenbach
Copy link
Copy Markdown
Contributor

Thanks for your contribution!

@asuessenbach asuessenbach merged commit 7923a22 into KhronosGroup:main Feb 18, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants