Skip to content

Conversation

@tgaldes
Copy link

@tgaldes tgaldes commented Nov 20, 2018

No description provided.

Copy link
Contributor

@accelerated accelerated left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thx! You need to remove the const qualifier from the rvalue constructor.

@mfontanini mfontanini merged commit 40ee64c into mfontanini:master Dec 16, 2018
@accelerated
Copy link
Contributor

accelerated commented Dec 17, 2018

Not sure this should have been merged. It was already adressed by PR#147. The raw array constructor is duplicated now.

@mfontanini
Copy link
Owner

Ugh true, how did this even compile?

@mfontanini
Copy link
Owner

So yeah no, this didn't compile https://travis-ci.org/mfontanini/cppkafka/jobs/468738999

@mfontanini
Copy link
Owner

For future reference @accelerated, please don't create a PR that duplicates a fix that another outstanding PR is already fixing. This is still on me for not double checking though.

Sorry @tgaldes that your commit had to be reverted!

@accelerated
Copy link
Contributor

Sure, but i mentioned it in my PR tho. Also the other one had been outstanding for a month and i assumed @tgaldes had given up on it.

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