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

Usage modern C++ features on JSON modules #4085

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

cngzhnp
Copy link

@cngzhnp cngzhnp commented Jul 18, 2023

Starting from this module, I want to use modern C++ features such as:

  • Use nullptr instead of 0.
  • Use auto instead of iterator type itself.
  • Use default keyword instead of copy constructor, destructor implementation
  • Use override keyword if necessary.
  • Use using keyword instead of typedef.
  • Use proper functions like empty() call instead of object.size() == 0

@teksturi
Copy link
Contributor

This is little bit old PR already but I still answer this. It does not make much sense to change style for just small portion of code base. It would be much better to do just one change at time. Example this

  • Use proper functions like empty() call instead of object.size() == 0

could be done to whole repo. It would be much more easy to accept those kind of changes.

@matejk
Copy link
Contributor

matejk commented Dec 22, 2023

#4354

@cngzhnp
Copy link
Author

cngzhnp commented Dec 23, 2023

This is little bit old PR already but I still answer this. It does not make much sense to change style for just small portion of code base. It would be much better to do just one change at time. Example this

  • Use proper functions like empty() call instead of object.size() == 0

could be done to whole repo. It would be much more easy to accept those kind of changes.

Hello @teksturi, sorry for late answer.

However, my intention was not just a change into small portion of codebase like this, also I am not fan of inconsistent things. I just want to split up code review based on submodules. Creating a PR with 100 files changed which was not easy to review and apply changes. So, doing it step by step would be much better idea at least for me at that time.

@matejk matejk deleted the branch pocoproject:devel April 15, 2024 11:20
@matejk matejk closed this Apr 15, 2024
@andrewauclair
Copy link
Contributor

andrewauclair commented Apr 18, 2024

@matejk did you mean to delete devel? Doesn't seem right. Nevermind. I just saw the discussion post. Had way too many notifications to go through.

@matejk
Copy link
Contributor

matejk commented Apr 20, 2024

@cngzhnp, would you be willing to rebase this PR to main?

@matejk matejk added this to the Release 1.14.0 milestone Apr 20, 2024
@matejk matejk reopened this Apr 22, 2024
bas524 and others added 2 commits April 29, 2024 10:54
…e encoded words (pocoproject#4542)

* fix bug pocoproject#4535
RFC 2047 decodeWord

* modify logic
uses separate string which contains simbols between chunks and if this
string contains only space ot \n or \t or \v than trim it
@cngzhnp cngzhnp force-pushed the refactor/modern_cpp_usage_on_json_module branch from fb275a1 to d3b7947 Compare May 5, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

7 participants