forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
backport: bitcoin#21613 -Wdocumentation and related fixes #6113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
PastaPastaPasta
merged 3 commits into
dashpay:develop
from
knst:bp-21613-Wdocumentation
Jul 19, 2024
Merged
backport: bitcoin#21613 -Wdocumentation and related fixes #6113
PastaPastaPasta
merged 3 commits into
dashpay:develop
from
knst:bp-21613-Wdocumentation
Jul 19, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a4e970a build: enable -Wdocumentation if suppressing external warnings (fanquake) 3b0078f doc: fixup -Wdocumentation issues (fanquake) c6edcf1 build: suppress libevent warnings if supressing external warnings (fanquake) Pull request description: Enable `-Wdocumentation` by taking advantage of our `--enable-suppress-external-warnings` flag. Most of the CIs are using this flag now, so any regressions should be caught. This also required modifying libevents flags when suppressing warnings, as depending on the version being built against, that could generate a large number of warnings. i.e: ```bash In file included from httpserver.cpp:34: In file included from ./support/events.h:12: /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:464:11: warning: parameter 'req' not found in the function declaration [-Wdocumentation] @param req a request object ^~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:465:11: warning: parameter 'databuf' not found in the function declaration [-Wdocumentation] @param databuf the data chunk to send as part of the reply. ^~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:467:11: warning: parameter 'call' not found in the function declaration [-Wdocumentation] @param call back's argument. ^~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:939:4: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated; you probably want to use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:946:1: note: add a deprecation attribute to the declaration to silence this warning char *evhttp_decode_uri(const char *uri); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:979:5: warning: declaration is marked with '@deprecated' command but does not have a deprecation attribute [-Wdocumentation-deprecated-sync] @deprecated This function is deprecated as of Libevent 2.0.9. Use ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:987:1: note: add a deprecation attribute to the declaration to silence this warning int evhttp_parse_query(const char *uri, struct evkeyvalq *headers); ^ __AVAILABILITY_INTERNAL_DEPRECATED /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: warning: parameter 'query_parse' not found in the function declaration [-Wdocumentation] @param query_parse the query portion of the URI ^~~~~~~~~~~ /usr/local/Cellar/libevent/2.1.12/include/event2/http.h:1002:11: note: did you mean 'uri'? @param query_parse the query portion of the URI ^~~~~~~~~~~ uri 69 warnings generated. ``` Note that a lot of these have already been fixed upstream. ACKs for top commit: laanwj: Concept and code review ACK a4e970a practicalswift: cr ACK a4e970a: automatic compiler feedback comes sooner and is more reliable than manual reviewer feedback jonatack: Light ACK a4e970a skimmed the changes, clang 11 build is clean with the change, verified -Wdocumentation build warnings with this change when a doc fix was reverted Tree-SHA512: 57a1e30cffcc8bcceee72d85f58ebe29eae525861c70acb237541bd480c51ede89875c033042c0af376fdbb49fb7f588ef9282a47c6e78f9d4501c41f1b21eb6
UdjinM6
approved these changes
Jul 15, 2024
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compiles with no issues
utACK 252ffee
PastaPastaPasta
approved these changes
Jul 19, 2024
Member
PastaPastaPasta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 252ffee
knst
added a commit
to knst/dash
that referenced
this pull request
Jul 23, 2024
It happened due to 2 PRs merged one after each other without rebasing: - enabling -Wdocumentation in bitcoin#21631 (dashpay#6113) - renaming param while doxygen comment is forgotten in dashpay#6114
5 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
Jul 23, 2024
…o wallet bebf391 fix: build with -Wdocumentation - rename param pwallet to wallet (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented CI failures like that: https://gitlab.com/dashpay/dash/-/jobs/7400661581 It happened due to 2 PRs merged one after each other without rebasing: - enabling -Wdocumentation in bitcoin#21631 (#6113) - renaming param while doxygen comment is forgotten in #6114 ## What was done? It contains changes from #6133 Thanks Vijay for spotting issue, this PR is DNM if 6133 will get merged faster. ## How Has This Been Tested? See CI ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK bebf391 PastaPastaPasta: utACK bebf391 Tree-SHA512: 34265f09098c04593a9d52709e5b8aed8460248762655945e5c86a65adb9aa49ab6f0bdb21cd907d353fe6b10e1ff2e07d05528166bf8e2140eb2c9ea1acbd33
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was done?
Backport bitcoin#21613 to enable -Wdocumentation and related fixes for dash code
How Has This Been Tested?
Build with clang compiler and see no failures anymore with
--enable-werrorBreaking Changes
N/A
Checklist: