You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2022/11/25 09:20:23 UTC
[GitHub] [qpid-proton] DreamPearl opened a new pull request, #382: PROTON-2654: [cpp] Tests for newly added map constructors
DreamPearl opened a new pull request, #382:
URL: https://github.com/apache/qpid-proton/pull/382
[PROTON-2654](https://issues.apache.org/jira/browse/PROTON-2654)
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#discussion_r1039811631
##########
cpp/src/map_test.cpp:
##########
@@ -114,6 +114,23 @@ void test_value() {
ASSERT_THROWS(conversion_error, m.value(bad));
}
+void test_copy_constructor() {
+ std::map<string, scalar> sm;
+ sm["a"] = 2;
+ sm["b"] = 3;
Review Comment:
or even (syntax might not be 100% correct):
`auto m = std::map<string,scalar>{{"a", 2}, {"b", 3}};`
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] codecov-commenter commented on pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#issuecomment-1327296304
# [Codecov](https://codecov.io/gh/apache/qpid-proton/pull/382?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#382](https://codecov.io/gh/apache/qpid-proton/pull/382?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (49a25a4) into [main](https://codecov.io/gh/apache/qpid-proton/commit/c9d8caa07a229f3d255159316fa55441ff638752?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c9d8caa) will **decrease** coverage by `0.10%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## main #382 +/- ##
==========================================
- Coverage 69.69% 69.59% -0.11%
==========================================
Files 370 370
Lines 69869 69883 +14
==========================================
- Hits 48696 48633 -63
- Misses 21173 21250 +77
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-proton/pull/382?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [cpp/src/map\_test.cpp](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y3BwL3NyYy9tYXBfdGVzdC5jcHA=) | `100.00% <100.00%> (ø)` | |
| [ruby/lib/core/delivery.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS9kZWxpdmVyeS5yYg==) | `57.57% <0.00%> (-36.37%)` | :arrow_down: |
| [ruby/lib/core/tracker.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29yZS90cmFja2VyLnJi) | `55.55% <0.00%> (-33.34%)` | :arrow_down: |
| [ruby/lib/codec/data.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvZGF0YS5yYg==) | `72.60% <0.00%> (-23.48%)` | :arrow_down: |
| [ruby/lib/handler/messaging\_adapter.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvaGFuZGxlci9tZXNzYWdpbmdfYWRhcHRlci5yYg==) | `89.85% <0.00%> (-7.25%)` | :arrow_down: |
| [ruby/spec/spec\_helper.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9zcGVjL3NwZWNfaGVscGVyLnJi) | `58.49% <0.00%> (-5.67%)` | :arrow_down: |
| [ruby/lib/codec/mapping.rb](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cnVieS9saWIvY29kZWMvbWFwcGluZy5yYg==) | `86.02% <0.00%> (-3.23%)` | :arrow_down: |
| [c/tests/threaderciser.c](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Yy90ZXN0cy90aHJlYWRlcmNpc2VyLmM=) | `83.68% <0.00%> (-0.06%)` | :arrow_down: |
| [c/src/proactor/epoll.c](https://codecov.io/gh/apache/qpid-proton/pull/382/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Yy9zcmMvcHJvYWN0b3IvZXBvbGwuYw==) | `86.28% <0.00%> (+0.16%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#issuecomment-1327200663
@astitcher @gemmellr
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] gemmellr commented on pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
gemmellr commented on PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#issuecomment-1327291906
Looks good to me, but I'll leave it to someone with more of a clue to merge in case I'm wrong :)
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#discussion_r1039808575
##########
cpp/src/map_test.cpp:
##########
@@ -114,6 +114,23 @@ void test_value() {
ASSERT_THROWS(conversion_error, m.value(bad));
}
+void test_copy_constructor() {
+ std::map<string, scalar> sm;
+ sm["a"] = 2;
+ sm["b"] = 3;
Review Comment:
Wouldn't this be clearer as:
`std::map<string, scalar> m = {{"a", 2}, {"b", 3}};`
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a diff in pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
astitcher commented on code in PR #382:
URL: https://github.com/apache/qpid-proton/pull/382#discussion_r1039820733
##########
cpp/src/map_test.cpp:
##########
@@ -114,6 +114,23 @@ void test_value() {
ASSERT_THROWS(conversion_error, m.value(bad));
}
+void test_copy_constructor() {
+ std::map<string, scalar> sm;
+ sm["a"] = 2;
+ sm["b"] = 3;
+ proton::map<string, scalar> m(sm);
+ ASSERT_EQUAL(scalar(2), m.get("a"));
+ ASSERT_EQUAL(scalar(3), m.get("b"));
+ ASSERT_EQUAL(2U, m.size());
+}
+
+void test_initializer_list_constructor() {
+ proton::map<string, scalar> m = {{"a", 2}, {"b", 3}};
Review Comment:
as above this could be:
`auto m = proton::map<string,scalar>{{"a", 2}, {"b", 3}};`
Not sure this is definitely better, but perhaps more idiomatic post c++11.
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher merged pull request #382: PROTON-2654: [cpp] Tests for newly added map constructors
Posted by GitBox <gi...@apache.org>.
astitcher merged PR #382:
URL: https://github.com/apache/qpid-proton/pull/382
--
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: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org