Skip to content
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

Fix for PROTON-1517 #110

Closed
wants to merge 1 commit into from
Closed

Conversation

aikchar
Copy link

@aikchar aikchar commented Jul 18, 2017

Fixes the bug reported in PROTON-1517.

@ssorj
Copy link
Contributor

ssorj commented Dec 8, 2017

@aikchar , I applied this patch. Please close when you get a chance.

2b03394

@codecov-io
Copy link

Codecov Report

Merging #110 into master will decrease coverage by 0.35%.
The diff coverage is 68.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   78.67%   78.31%   -0.36%     
==========================================
  Files         231      232       +1     
  Lines       30546    30687     +141     
  Branches     2903     2927      +24     
==========================================
+ Hits        24033    24034       +1     
- Misses       4872     4912      +40     
- Partials     1641     1741     +100
Impacted Files Coverage Δ
proton-c/bindings/cpp/src/scalar_test.hpp 100% <ø> (ø)
examples/c/send-ssl.c 76.28% <ø> (ø) ⬆️
examples/c/send.c 75.82% <ø> (ø) ⬆️
proton-c/bindings/cpp/src/connection_driver.cpp 77.77% <ø> (ø)
examples/cpp/multithreaded_client.cpp 81.81% <ø> (ø) ⬆️
examples/c/send-abort.c 76.34% <ø> (ø) ⬆️
examples/c/direct.c 83.63% <ø> (ø) ⬆️
proton-c/bindings/cpp/src/types_internal.hpp 94.73% <ø> (ø)
proton-c/src/core/message.c 59.53% <ø> (-0.78%) ⬇️
examples/cpp/ssl_client_cert.cpp 81.08% <ø> (ø) ⬆️
... and 71 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ff005f...a4dafa2. Read the comment docs.

@dmitrytokarev
Copy link

@ssorj why didn't you just merge this PR?
Now @aikchar doesn't get credit for his work.

@ssorj
Copy link
Contributor

ssorj commented Dec 9, 2017

@dmitrytokarev , I was failing at making it merge the way I wanted. I will try to figure out the right sequence of commands so I get it right next time.

@jirkadanek
Copy link
Contributor

whoa, codecov.io has gone crazy... this report above includes about 10 recent commits to master, not just this one...

@dmitrytokarev Is committing as myself, but setting git commit --author "Other person <other@person.edu>" acceptable GitHub practice? Or doing a final commit --amend with the --author set? This is what I intended to do (in my personal project...)

@dmitrytokarev
Copy link

@jdanekrh well it doesn't really matter how it's done as long as community contributions are authored and recognized. Some people may show case their github accounts on their resumes, so if they don't get credit for their work, they have nothing to showcase.

@asfgit asfgit closed this in 0c2cbaf Mar 22, 2018
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.

5 participants