You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/10/11 19:40:10 UTC
[GitHub] [couchdb] jaydoane opened a new pull request #3782: Eliminate compiler warnings
jaydoane opened a new pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
- Replace unused variable with underscore
- Add nowarn_export_all compiler option
- Use STACKTRACE macro
<!-- Please give a short brief for the pull request,
what problem it solves or how it makes things better. -->
## Testing recommendations
```
make
```
Should no longer display e.g.
```
src/chttpd/src/chttpd_node.erl:35: Warning: variable 'Node' is unused
src/couch_pse_tests/src/cpse_test_purge_seqs.erl:14: Warning: export_all flag enabled - all functions will be exported
src/smoosh/src/smoosh_server.erl:251: Warning: erlang:get_stacktrace/0: deprecated; use the new try/catch syntax for retrieving the stack backtrace
C
```
<!-- Describe how we can test your changes.
Does it provides any behaviour that the end users
could notice? -->
## Related Issues or Pull Requests
<!-- If your changes affects multiple components in different
repositories please put links to those issues or pull requests here. -->
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] iilyak commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
iilyak commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r726968772
##########
File path: src/couch_replicator/src/couch_replicator_doc_processor.erl
##########
@@ -875,7 +875,17 @@ setup() ->
teardown(Pid) ->
- exit(Pid, kill).
+ exit(Pid, kill),
Review comment:
You also can use `test_util:stop_sync(Pid, kill, 5000)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r727302316
##########
File path: src/couch_replicator/src/couch_replicator_doc_processor.erl
##########
@@ -875,7 +875,17 @@ setup() ->
teardown(Pid) ->
- exit(Pid, kill).
+ exit(Pid, kill),
Review comment:
Seems like the perfect use of `test_util:stop_sync/3`. Thanks for pointing that out, @iilyak!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r726736831
##########
File path: src/chttpd/src/chttpd_node.erl
##########
@@ -32,7 +32,7 @@ handle_node_req(#httpd{path_parts=[_, <<"_local">>]}=Req) ->
handle_node_req(#httpd{path_parts=[A, <<"_local">>|Rest]}=Req) ->
handle_node_req(Req#httpd{path_parts=[A, node()] ++ Rest});
% GET /_node/$node/_versions
-handle_node_req(#httpd{method='GET', path_parts=[_, Node, <<"_versions">>]}=Req) ->
+handle_node_req(#httpd{method='GET', path_parts=[_, _, <<"_versions">>]}=Req) ->
Review comment:
I originally used `_Node` but then the line was longer than I liked, so I just changed it to `_`. But you are right, it should be the former for consistency, so I've updated the commit to use `_Node` and changed the commit message appropriately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#issuecomment-941091086
@jaydoane good find, that flaky test has been plaguing us for a quite a while
Good idea to use stop_sync, @iilyak
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane commented on pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#issuecomment-940674564
@nickva I was seeing some test failures due to this error:
```
*** context setup failed ***
**in function couch_replicator_doc_processor:setup/0 (src/couch_replicator_doc_processor.erl, line 872)
**error:{badmatch,{error,{already_started,<0.4946.0>}}}
```
so I pushed another commit to address that issue.
Seems like it did the trick, but would appreciate a quick sanity check that it is a good solution for the problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r726701467
##########
File path: src/chttpd/src/chttpd_node.erl
##########
@@ -32,7 +32,7 @@ handle_node_req(#httpd{path_parts=[_, <<"_local">>]}=Req) ->
handle_node_req(#httpd{path_parts=[A, <<"_local">>|Rest]}=Req) ->
handle_node_req(Req#httpd{path_parts=[A, node()] ++ Rest});
% GET /_node/$node/_versions
-handle_node_req(#httpd{method='GET', path_parts=[_, Node, <<"_versions">>]}=Req) ->
+handle_node_req(#httpd{method='GET', path_parts=[_, _, <<"_versions">>]}=Req) ->
Review comment:
Below in the next clause we used `_Node` we could stick with the same clause. Alternatively, if it looks better to you, we could make both `_` as it's fairly obvious from the comment in line 34 what the path elements are.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r727302316
##########
File path: src/couch_replicator/src/couch_replicator_doc_processor.erl
##########
@@ -875,7 +875,17 @@ setup() ->
teardown(Pid) ->
- exit(Pid, kill).
+ exit(Pid, kill),
Review comment:
Seems like the perfect use of `test_util:stop_sync/3`. Thanks for pointing that out, @iilyak!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a change in pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782#discussion_r726701467
##########
File path: src/chttpd/src/chttpd_node.erl
##########
@@ -32,7 +32,7 @@ handle_node_req(#httpd{path_parts=[_, <<"_local">>]}=Req) ->
handle_node_req(#httpd{path_parts=[A, <<"_local">>|Rest]}=Req) ->
handle_node_req(Req#httpd{path_parts=[A, node()] ++ Rest});
% GET /_node/$node/_versions
-handle_node_req(#httpd{method='GET', path_parts=[_, Node, <<"_versions">>]}=Req) ->
+handle_node_req(#httpd{method='GET', path_parts=[_, _, <<"_versions">>]}=Req) ->
Review comment:
Below in the next clause we used `_Node` we could stick with the same pattern here too?. Alternatively, if it looks better, we could make both `_` as it's fairly obvious from the comment in line 34 what the path elements are.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane merged pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane merged pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane merged pull request #3782: Eliminate compiler warnings
Posted by GitBox <gi...@apache.org>.
jaydoane merged pull request #3782:
URL: https://github.com/apache/couchdb/pull/3782
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org