<p> Attention is currently required from: Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/15987">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987/comment/99164139_1288a8f7">Patch Set #1, Line 7:</a> <code style="font-family:monospace,monospace">res_stasis_playback</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This needs to follow the commit message formatting[1] […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987/comment/ec19a217_7b391d5e">Patch Set #3, Line 7:</a> <code style="font-family:monospace,monospace">res_stasis_playback: Send PlaybackFinish event only once for a list of sounds, when we have errors</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">These should be wrapped at 80 characters.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987?tab=comments">Patch Set #1:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The rest-api/api-docs/recordings.json file needs to be updated with this new state. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Updated rest-api/api-docs/playbacks.json with the new state.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987?tab=comments">Patch Set #2:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I had additional comments on my previous review regarding updating the REST API documentation, but a […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sorry Joshua, I missed the error/failure comment.</p><p style="white-space: pre-wrap; word-wrap: break-word;">On the existing states for the Playback, I didn't found the error/failure state, that's why I added one:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br>/*! State of a playback operation */<br>enum stasis_app_playback_state {<br> /*! The playback has not started yet */<br> STASIS_PLAYBACK_STATE_QUEUED,<br> /*! The media is currently playing */<br> STASIS_PLAYBACK_STATE_PLAYING,<br> /*! The media is currently playing */<br> STASIS_PLAYBACK_STATE_PAUSED,<br> /*! The media is transitioning to the next in the list */<br> STASIS_PLAYBACK_STATE_CONTINUING,<br> /*! The media has stopped playing */<br> STASIS_PLAYBACK_STATE_COMPLETE,<br> /*! The playback was canceled. */<br> STASIS_PLAYBACK_STATE_CANCELED,<br> /*! The playback was stopped. */<br> STASIS_PLAYBACK_STATE_STOPPED,<br> /*! Enum end sentinel. */<br> STASIS_PLAYBACK_STATE_MAX,<br>};</pre><p style="white-space: pre-wrap; word-wrap: break-word;"><br>On the statis_app_recording.h, we have the STASIS_APP_RECORDING_STATE_FAILED state.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So, to keep it coherent with on the Playback side, I will change from STASIS_PLAYBACK_STATE_ERROR to STASIS_PLAYBACK_STATE_FAILED.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> Patch updated with your suggestion. Thanks!</pre></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987?tab=comments">Patch Set #4:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Thinking further on this, I also think there should be a CHANGES entry[1] so ARI users are aware of […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Added, thanks for the link.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_stasis_playback.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987/comment/eb1c5fa5_731df8a0">Patch Set #1, Line 111:</a> <code style="font-family:monospace,monospace"> type = "PlaybackFinished"; </code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Redness</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15987/comment/99d9eefc_2340252d">Patch Set #1, Line 208:</a> <code style="font-family:monospace,monospace"> return "error"; </code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Redness</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15987">change 15987</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15987"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4c2e5921cc597702513af0d7c6c2c982e1798322 </div>
<div style="display:none"> Gerrit-Change-Number: 15987 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Andre Barbosa <andre.emanuel.barbosa@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 16 Jun 2021 12:21:51 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>