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

Issue 111: change all examples to eliminate deprecated class. #112

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jplflyer
Copy link

Use of the curlpp::Cleanup class is commented as deprecated in the include file, but it's being used in all the examples. This commit changes all of them to use

curlpp::initialize();
...
curlpp::terminate();

In a few places, I also fixed inconsistent indentation within the file, but I didn't try to fix different indentation styles across all the examples.

…ew inconsistent indentation issues while at it.
@sgallou sgallou requested review from sgallou and removed request for sgallou October 29, 2020 16:38
@sgallou
Copy link
Collaborator

sgallou commented Oct 29, 2020

Hi @jplflyer , your fix is accorded to the Cleanup class documentation, so I'm OK with it.
But why your commit have so many differences (spaces, empty lines, line returns...) ? Can you revert it, please ?
Also please remove commented code, and unnecessary comment. Just keep the calls to initialize() and terminate().

Thanks for your contribution

@jplflyer
Copy link
Author

I entirely removed the code I commented out. I'd left it in there for historical purposes, but it's gone, with the associated comments.

It looks like my IDE automatically removes trailing whitespace in C++ code. I didn't know it was doing that. That is probably the explanation for many of the extra changes.

I don't have time to spend another 2 hours on this right now. Deadlines at work and deadlines at night. Perhaps you're happy with this the way it is. Perhaps you will leave this dangling, and if I ever get time, I can figure out how to leave your trailing whitespaces alone, but that won't be this week.

Copy link
Collaborator

@sgallou sgallou left a comment

Choose a reason for hiding this comment

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

Thanks for the removing of comments. But I prefer to wait you remove all remaining spaces differences, because it can create some merge conflicts when merging with other contributors work.
Thanks for your understanding.

@@ -100,5 +101,7 @@ int main(int argc, char *argv[])
std::cout << e.what() << std::endl;
}

curlpp::terminate();

return EXIT_FAILURE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you want to return retVal here ?

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