[Asterisk-code-review] func_aes: Avoid incorrect error message on load. (asterisk[17])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14010 )

Change subject: func_aes: Avoid incorrect error message on load.
......................................................................


Patch Set 2:

> Patch Set 2: Code-Review+1
> 
> > Patch Set 2:
> > 
> > > we should also check for an error result from all calls to res_crypto functions, return an error from the dialplan functions if appropriate
> > 
> > What does this mean for me? (a) Do I have to add that, to get this change passed, (b) do you take that over in a new change (as feature request), (c) or do you want to take over this change here and add that? Guys, think about the contributor and how to ease his live. Not everyone is able to git, git review, or even git rebase. Tell him what *exactly* is expected from the contributor, how he and his change can continue, *and* why you cannot do it yourself. That is not that difficult if you try to put yourself into the view of the contributor.
> > 
> > Furthermore, it would be cool if the Asterisk Team could feedback in the master branch and not in one of the cherry-picks . Although I get that E-mail notifications, I had to search now in which branch the feedback actually was. Yes, of course, if the cherry-picks are different, feedback there. However, for a change which is the same for all cherry picks, please, feedback in the master branch. That saves time. And time is code.
> 
> Corey is a community member and contributor, just like you.

Whoops, my response was incomplete. Let me finish it up!

For a lot of contributors and reviewers when we respond we generally expect the individual who put up the review to take care of the feedback, and usually if something makes sense to be a separate review we will say so. If the individual is not open to or able to update the review that's perfectly fine and generally someone else may take it over if it makes sense. A comment just needs to be left, or it can be left alone and after a period of time someone like myself will ask if the person is able to update or wants someone else to take it over. Where that generally does not happen is when it is a large new feature that someone has proposed.

For the specific comment that Corey had on the func_aes behavior when it fails, it's fine to not have it as part of this review or at all. It would be an improvement and as noone has brought it up before it does not appear to be impacting many. Corey just saw an area where this should have been done originally and it wasn't while reviewing this change.

As for commenting on master reviews only - there is no enforcement possible in Gerrit to do so, it can certainly be asked of people but there is no way to force people to do it. Everyone is welcome to review, contribute, and participate how they wish.


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14010
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: I0b99b8468cbeb3b0eab23069cbd64062ef885ffc
Gerrit-Change-Number: 14010
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-CC: Corey Farrell <git at cfware.com>
Gerrit-Comment-Date: Tue, 31 Mar 2020 13:13:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200331/6b115bd1/attachment-0001.html>