Skip to content

THRIFT-5602: Use std::unique_ptr instead of boost::scoped_array - #2630

Merged
emmenlau merged 1 commit into
apache:masterfrom
kou:cpp-scoped-array-to-unique-ptr
Sep 10, 2022
Merged

emmenlau merged 1 commit into
apache:masterfrom
kou:cpp-scoped-array-to-unique-ptr

Conversation

@kou

@kou kou commented Jul 1, 2022

Copy link
Copy Markdown
Member

It seems that Apache Thrift wants to drop Boost dependency eventually.
See also:

# Remove the following once lib/cpp no longer depends on boost headers:
include(BoostMacros)

# Remove the following once lib/cpp no longer depends on boost headers:
include(BoostMacros)

This is a step for it. We don't need to use boost::scoped_array because
we can use std::unique_ptr. We require C++11 or later and std::unique_ptr
can be used with C++11 or later.

  • Did you create an Apache Jira ticket? (not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

@kou
kou force-pushed the cpp-scoped-array-to-unique-ptr branch from f083c7a to aeec774 Compare July 1, 2022 06:37
@emmenlau

emmenlau commented Sep 9, 2022

Copy link
Copy Markdown
Member

Dear @kou , your changes are highly appreciated! But since the build failed in Appveyor and Travis, could you kindly rebase on latest master and force push again, to see if the build errors go away? If not, we need to see if they are related to your changes...

Client: cpp

We can use std::unique_ptr because we require C++11 or later.
@kou
kou force-pushed the cpp-scoped-array-to-unique-ptr branch from aeec774 to d6a42e1 Compare September 9, 2022 14:25
@kou

kou commented Sep 9, 2022

Copy link
Copy Markdown
Member Author

Rebased. I hope that all CI jobs are green...

@kou

kou commented Sep 9, 2022

Copy link
Copy Markdown
Member Author

The AppVeyor job is green but some Travis CI jobs are still failed. But the same Travis CI jobs on master are failed:

It seems that these Travis CI failures aren't related to this change.

@emmenlau

Copy link
Copy Markdown
Member

Yep, I agree, the errors are unrelated, so your changes look good! Thanks for pushing this forward. I'll test it locally in my CI and then merge.

@emmenlau

Copy link
Copy Markdown
Member

I've tested the changes also against our internal build with some different compilers, and run test suites on Linux, Windows and MacOS. Everything looks good, so merging. Thanks for the nice cleanup!

@emmenlau
emmenlau merged commit c96c044 into apache:master Sep 10, 2022
@kou
kou deleted the cpp-scoped-array-to-unique-ptr branch September 10, 2022 08:05
@kou

kou commented Sep 10, 2022

Copy link
Copy Markdown
Member Author

Thanks!

@emmenlau

Copy link
Copy Markdown
Member

Thanks to you! If you have more such changes, please keep them coming!

@cfriedt

cfriedt commented Oct 1, 2022 •

Copy link
Copy Markdown
Contributor

Great change @kou - standards FTW! 👍

cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 1, 2022
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682
apache/thrift#2689

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
cfriedt pushed a commit to zephyrproject-rtos/gsoc-2022-thrift that referenced this pull request Oct 2, 2022
With this, we're starting to clean up the commit history a bit.
See #147  for details.

apache/thrift#2687
apache/thrift#2681
apache/thrift#2680
apache/thrift#2630
apache/thrift#2682
apache/thrift#2689

Signed-off-by: Christopher Friedt <cfriedt@meta.com>
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