From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id ECB1F469719 for ; Tue, 17 Mar 2020 17:19:54 +0300 (MSK) Date: Tue, 17 Mar 2020 14:19:53 +0000 From: Nikita Pettik Message-ID: <20200317141953.GB3221@tarantool.org> References: <20200313140612.29775-1-olegrok@tarantool.org> <5905e1da-c4a6-6d9b-d17c-a2dbe1d14033@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <5905e1da-c4a6-6d9b-d17c-a2dbe1d14033@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v3] box: allow to retrieve the last generated value of sequence List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: Oleg Babin , tarantool-patches@dev.tarantool.org On 17 Mar 00:04, Vladislav Shpilevoy wrote: > >>> diff --git a/src/box/sequence.h b/src/box/sequence.h > >>> index a164da9af..8c442872a 100644 > >>> --- a/src/box/sequence.h > >>> +++ b/src/box/sequence.h > >>> @@ -171,10 +171,11 @@ sequence_data_iterator_create(void); > >>>    * Get last element of given sequence. > >>>    * > >>>    * @param seq sequence to get value from. > >>> - * @retval last element of sequence. > >>> + * @result value of sequence. > >> > >> 3. According to doxygen doc, @result is the same as @return. So > >> this comment says "return value of sequence". I think you wanted > >> to refer to 'result' parameter. For that doxygen provides command > >> @param. > >> > >>> + * Return 0 on success, -1 if sequence is not initialized. > >> > >> 4. Doxygen way of documenting return values is either > >> > >>      @retval Meaning. > >>      @retval Meaning. > >> > >> Or > >> > >>      @return/returns Meaning. > >> > >>>    */ > > > > I looked how it was done for "sequence_next" and have changed in the same manner: > > | /** > > | * Get last element of given sequence. > > | * > > | * @param seq sequence to get value from. > > | * On success, return 0 and assign the current value of the > > | * sequence to @result, otherwise return -1 and set diag. > > | */ > > The problem is that sequence_next() comment also is not a > valid doxygen syntax. So copy-paste here is not a solution. > > But ok, I guess it is time to give up on trying to make the > comments correct everywhere. > > LGTM. > > Nikita, please, do a second review. Okay, np. Oleg, could you please re-send patch with all updates?