You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by Arjun Ray <ar...@gmail.com> on 2021/04/14 16:28:36 UTC

Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

On Tue, 20 Oct 2020 00:11:24 -0400, I wrote:

| Compiling the ActiveMQ-CPP distribution generates hundreds of warnings
| on using std::auto_ptr, which has been deprecated since C++11 nearly a
| decade ago, and in fact has been removed altogether in C++17.
| 
| Is there a show-stopping issue with replacing std::auto_ptr with
| std::unique_ptr?  I couldn't find any discussion of this. My resources
| are limited to the Linux platform (specifically Ubuntu), where I find
| that there are very few pain points in the replacement.

I've since found that a 'discussion' of sorts took place several years
ago.  Someone raised a JIRA ticket for precisely this issue, and
posted a proposed patch, whereupon the ticket was summarily closed as
"Won't fix", without any explanation.

 https://issues.apache.org/jira/browse/AMQCPP-628

| With these changes, the library compiles and all tests pass.  I don't
| have the resources to verify this with other compilers or on other
| platforms, which is why I haven't yet investigated how to submit a
| pull request.

It seems that a PR would meet the same fate.  Forking may be the
better option for those looking to improve the codebase.

Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

Posted by Matt Pavlovich <ma...@gmail.com>.

> On Apr 20, 2021, at 9:13 PM, Arjun Ray <ar...@gmail.com> wrote:

> Getting rid of std::auto_ptr is only staving off an immediate problem
> (which really should have been addressed years ago, considering how
> simple the fix is.)  But there are other serious issues with the code
> base that also need to be addressed, sooner rather than later.

Understood. I think we can take it in phases and seek to attract additional C++ developers to provide insight to best practices with modern changes to the language and standard libraries.

> If something as not much more than cosmetic as replacing std::auto_ptr
> doesn't find favor with the gatekeepers, I don't fancy the chances for
> any serious changes.

Keeping PR’s and changes small allows changes to be reviewed by a wider audience quickly and I think you’d see velocity pick up on the wider changes. 

> | 1. Create a clone of this git repo activemq-cpp <https://github.com/apache/activemq-cpp>
> 
> This clone is in github itself, right?

Yes, clone to a repository in under your profile and PR’s are automatically detected and a click away.

> | 2. Create a branch (using the JIRA name is good ie AMQCPP-664)
> 
> And this is in a local copy of my github clone, right?

Rough sketch of the git flow:

1. Clone the repo into a repository in your GitHub profile
2. Checkout that git repo locally 

   $ git clone https://github.com/yourprofile/activmeq.git <https://github.com/yourprofile/activmeq.git>

3. Add upstream as a remote:

$ git remote -v  (Observe only origin exists pointing to your personal repo)
$ git remote add upstream https://github.com/activemq/activemq.git
$ git remote -v  (Observe the upstream apache repo is listed)

Create a branch 
4. $ git checkout -b AMQ-xxxx
5. Make changes.. and commit
6. $ git push origin AMQ-xxxx
7. You can now click on Create Pull Request in GitHub 




Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

Posted by Arjun Ray <ar...@gmail.com>.
[Default] On Fri, 16 Apr 2021 08:57:06 -0500, Matt Pavlovich
<ma...@gmail.com> wrote:

| I anticipate an updated CMS library would be very useful to many 
| ActiveMQ users for years to come.

The utility to users is beyond question.  The issue is the interest of
the developers/gatekeepers, who can forestall or prevent improvements,
by simply doing nothing.  If they have genuinely lost interest...

There are already 9 outstanding PRs, going back several years.

Getting rid of std::auto_ptr is only staving off an immediate problem
(which really should have been addressed years ago, considering how
simple the fix is.)  But there are other serious issues with the code
base that also need to be addressed, sooner rather than later.

If something as not much more than cosmetic as replacing std::auto_ptr
doesn't find favor with the gatekeepers, I don't fancy the chances for
any serious changes.
   
| 1. Create a clone of this git repo activemq-cpp <https://github.com/apache/activemq-cpp>

This clone is in github itself, right?

| 2. Create a branch (using the JIRA name is good ie AMQCPP-664)

And this is in a local copy of my github clone, right?

| 3. Make the code changes
| 4. Push the branch to your cloned repo.
| 5. GitHub will prompt you to create a Pull Request (PR) since it detects your repo is a clone of the upstream.
| 6. PR can now be reviewed and merged by others.

I've run into an issue with this procedure, that I'm trying to work
through.

-- 
:ar

Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

Posted by Matt Pavlovich <ma...@gmail.com>.

> On Apr 14, 2021, at 12:48 PM, Arjun Ray <ar...@gmail.com> wrote:
> 
> On Wed, 14 Apr 2021 12:05:22 -0500, you wrote:
> 
> | I知 in favor of updated the CMS to a more current C++, given the
> |  sustained interest in C++ and ActiveMQ.
> 
> In-project interest seems to have shifted to Artemis (and AMQP),
> making Classic a step-child within its own project.

ActiveMQ has a well established user base and significant continued investment by developers and end-users. I anticipate an updated CMS library would be very useful to many ActiveMQ users for years to come.

> | Generally, forking isn稚 great for anyone in the long term. 
> 
> I agree.  But what choices remain when "Won't fix" wihout explanation
> is the gatekeepers' response to an obvious improvement?
> 
> | Perhaps its best suited for a v4.x release and baseline against 
> | newer C++ libs?
> 
> This sounds like a good idea.  What is the procedure for this? 



1. Create a clone of this git repo activemq-cpp <https://github.com/apache/activemq-cpp>
2. Create a branch (using the JIRA name is good ie AMQCPP-664)
3. Make the code changes
4. Push the branch to your cloned repo.
5. GitHub will prompt you to create a Pull Request (PR) since it detects your repo is a clone of the upstream.
6. PR can now be reviewed and merged by others.

A brief guide here: https://activemq.apache.org/contributing <https://activemq.apache.org/contributing>

Thanks!
Matt Pavlovich


Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

Posted by Arjun Ray <ar...@gmail.com>.
On Wed, 14 Apr 2021 12:05:22 -0500, you wrote:

| I’m in favor of updated the CMS to a more current C++, given the
|  sustained interest in C++ and ActiveMQ.

In-project interest seems to have shifted to Artemis (and AMQP),
making Classic a step-child within its own project.
 
| Generally, forking isn’t great for anyone in the long term. 

I agree.  But what choices remain when "Won't fix" wihout explanation
is the gatekeepers' response to an obvious improvement?

| Perhaps its best suited for a v4.x release and baseline against 
| newer C++ libs?

This sounds like a good idea.  What is the procedure for this? 

Re: Replacing std::auto_ptr in ActiveMQ-CPP v.3.9.5

Posted by Matt Pavlovich <ma...@gmail.com>.
I’m in favor of updated the CMS to a more current C++, given the sustained interest in C++ and ActiveMQ.

Generally, forking isn’t great for anyone in the long term. Perhaps its best suited for a v4.x release and baseline against newer C++ libs?

-Matt Pavlovich

> On Apr 14, 2021, at 11:28 AM, Arjun Ray <ar...@gmail.com> wrote:
> 
> On Tue, 20 Oct 2020 00:11:24 -0400, I wrote:
> 
> | Compiling the ActiveMQ-CPP distribution generates hundreds of warnings
> | on using std::auto_ptr, which has been deprecated since C++11 nearly a
> | decade ago, and in fact has been removed altogether in C++17.
> | 
> | Is there a show-stopping issue with replacing std::auto_ptr with
> | std::unique_ptr?  I couldn't find any discussion of this. My resources
> | are limited to the Linux platform (specifically Ubuntu), where I find
> | that there are very few pain points in the replacement.
> 
> I've since found that a 'discussion' of sorts took place several years
> ago.  Someone raised a JIRA ticket for precisely this issue, and
> posted a proposed patch, whereupon the ticket was summarily closed as
> "Won't fix", without any explanation.
> 
> https://issues.apache.org/jira/browse/AMQCPP-628
> 
> | With these changes, the library compiles and all tests pass.  I don't
> | have the resources to verify this with other compilers or on other
> | platforms, which is why I haven't yet investigated how to submit a
> | pull request.
> 
> It seems that a PR would meet the same fate.  Forking may be the
> better option for those looking to improve the codebase.