[Asterisk-code-review] cdr.c: Eliminated simple RAII VAR usages. (asterisk[13])

Kevin Harwell asteriskteam at digium.com
Tue Oct 10 12:31:41 CDT 2017


Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/6722 )

Change subject: cdr.c: Eliminated simple RAII_VAR usages.
......................................................................


Patch Set 1:

> > > These are definitely not whitespace changes. Some of the exit
 > > paths now do not call ao2_cleanup() when the cleanup object is
 > > always going to be NULL.
 > >
 > > Right, but it's still just moving code around for the sake of it.
 > > These changes have no bearing on functionality. I'm not
 > necessarily
 > > against it (heck lots of the code could use some refactoring).
 > >
 > > I'm more wondering for changes like these if we need a coding
 > > guidelines rule similar to the whitespace one.
 > >
 > > Anyone else feel free to chime in with opinions.
 > 
 > The subsequent patches I'm going to post for CDR performance depend
 > upon this change already being done.  Otherwise there are
 > unnecessary
 > conflicts between branches which makes it harder to cherry-pick
 > changes.

So I agree with this and because of this I've wondered why whitespace changes are only allowed in master as it'll just make merging harder. Maybe it's time to remove that stipulation?

As is there are a few whitespace/formatting changes in this patch that would otherwise need to be removed.


-- 
To view, visit https://gerrit.asterisk.org/6722
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: I150505db307249a962987e7b941bdd369bb91f35
Gerrit-Change-Number: 6722
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 17:31:41 +0000
Gerrit-HasComments: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171010/6be9ccd5/attachment.html>


More information about the asterisk-code-review mailing list