[pacman-dev] [PATCH] Add documentation for transaction events
It is hard to decipher what the transaction events actually notify you
of, and
what parameters are passed to the callback function, without looking
at the
code. This patch adds documentation for the _pmtransevt_t enum in
order to
clarify what the event is for and what data is passed when the
callback is
called.
Signed-off-by: Sebastian Nowicki
On Sat, Apr 26, 2008 at 2:04 PM, Sebastian Nowicki
It is hard to decipher what the transaction events actually notify you of, and what parameters are passed to the callback function, without looking at the code. This patch adds documentation for the _pmtransevt_t enum in order to clarify what the event is for and what data is passed when the callback is called.
Signed-off-by: Sebastian Nowicki
Wow, awesome++. Thank you for doing this. Two issues though- your mailer is wrapping lines meaning this patch doesn't apply cleanly- you can either set your mailer so it doesn't rewrap things, or try to get git-send-email working which always works for me and prevents any line wrap errrors. My only other concern is blowing up alpm.h unnecessarily, but I don't see too many other ways of doing this. Perhaps we can get rid of all the blank comment lines in each desc section? These:
+ *
And get rid of the @brief tag and just move each first-line comment to the first line of the comment block? Doxygen also stops the brief comment automatically at the first '.' char, so we probably need to add those and fill out the punctuation elsewhere. So in short, this:
+ /** + * @brief Dependencies will be computed for a package + * + * No paramaters are passed + */
becomes:
+ /** Dependencies will be computed for a package. + * No parameters are passed. + */
(brief moved up, blank lines killed, periods added, s/paramater/parameter/g) Documentation is awesome to have though, so I really appreciate you taking the time to get all this down and written up. -Dan
--- lib/libalpm/alpm.h | 166 +++++++++++++++++++++++++++++++++++++++++++ ++++++++- 1 files changed, 165 insertions(+), 1 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index a7ea063..0c2da10 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -283,36 +283,200 @@ typedef enum _pmtransflag_t { PM_TRANS_FLAG_RECURSEALL = 0x10000 } pmtransflag_t;
-/* Transaction Events */ +/** + * @addtogroup alpm_trans + * @brief Transaction events + */ +/*@{*/ typedef enum _pmtransevt_t { + /** + * @brief Dependencies will be computed for a package + * + * No paramaters are passed + */ PM_TRANS_EVT_CHECKDEPS_START = 1, + + /** + * @brief Dependencies were computed for a package + * + * No paramaters are passed + */ PM_TRANS_EVT_CHECKDEPS_DONE, + + /** + * @brief File conflicts will be computed for a package + * + * No paramaters are passed + */ PM_TRANS_EVT_FILECONFLICTS_START, + + /** + * @brief File conflicts were computed for a package + * + * No paramaters are passed + */ PM_TRANS_EVT_FILECONFLICTS_DONE, + + /** + * @brief Dependencies will be resolved for target package + * + * No paramaters are passed + */ PM_TRANS_EVT_RESOLVEDEPS_START, + + /** + * @brief Dependencies were resolved for target package + * + * No paramaters are passed + */ PM_TRANS_EVT_RESOLVEDEPS_DONE, + + /** + * @brief Inter-conflicts will be checked for target package + * + * No paramaters are passed + */ PM_TRANS_EVT_INTERCONFLICTS_START, + + /** + * @brief Inter-conflicts were checked for target package + * + * No paramaters are passed + */ PM_TRANS_EVT_INTERCONFLICTS_DONE, + + /** + * @brief Package will be installed + * + * A pointer to the target package is passed to the callback + */ PM_TRANS_EVT_ADD_START, + + /** + * @brief Package was installed + * + * A pointer to the new package is passed to the callback + */ PM_TRANS_EVT_ADD_DONE, + + /** + * @brief Package will be removed + * + * A pointer to the target package is passed to the callback + */ PM_TRANS_EVT_REMOVE_START, + + /** + * @brief Package was removed + * + * A pointer to the removed package is passed to the callback + */ PM_TRANS_EVT_REMOVE_DONE, + + /** + * @brief Package will be upgraded + * + * A pointer to the upgraded package is passed to the callback + */ PM_TRANS_EVT_UPGRADE_START, + + /** + * @brief Package was upgraded + * + * A pointer to the new package, and a pointer to the old package is + * passed to the callback, respectively + */ PM_TRANS_EVT_UPGRADE_DONE, + + /** + * @brief Package waas extracted + * + * No paramaters are passed + */ PM_TRANS_EVT_EXTRACT_DONE, + + /** + * @brief Target package's integrity will be checked + * + * No paramaters are passed + */ PM_TRANS_EVT_INTEGRITY_START, + + /** + * @brief Target package's integrity was checked + * + * No paramaters are passed + */ PM_TRANS_EVT_INTEGRITY_DONE, + + /** + * @brief Target deltas's integrity will be checked + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_INTEGRITY_START, + + /** + * @brief Target delta's integrity was checked + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_INTEGRITY_DONE, + + /** + * @brief Deltas will be applied to packages + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_PATCHES_START, + + /** + * @brief Deltas were applied to packages + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_PATCHES_DONE, + + /** + * @brief Delta patch will be applied to target package + * + * The filename of the package and the filename of the patch is passed to + * the callback + */ PM_TRANS_EVT_DELTA_PATCH_START, + + /** + * @brief Delta patch was applied to target package + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_PATCH_DONE, + + /** + * @brief Delta patch failed to apply to target package + * + * No paramaters are passed + */ PM_TRANS_EVT_DELTA_PATCH_FAILED, PM_TRANS_EVT_SCRIPTLET_INFO, + + /** + * @brief Print URI + * + * The database's URI and the package's filename are passed to the + * callback + */ PM_TRANS_EVT_PRINTURI, + + /** + * @brief Files will be downloaded from a repository + * + * The repository's tree name is passed to the callback + */ PM_TRANS_EVT_RETRIEVE_START, } pmtransevt_t; +/*@}*/
/* Transaction Conversations (ie, questions) */ typedef enum _pmtransconv_t { -- 1.5.4.5
-- Sebastian Nowicki
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
On 27/04/2008, at 3:17 AM, Dan McGee wrote:
Wow, awesome++. Thank you for doing this. Two issues though- your mailer is wrapping lines meaning this patch doesn't apply cleanly- you can either set your mailer so it doesn't rewrap things, or try to get git-send-email working which always works for me and prevents any line wrap errrors.
Sorry, this was the first time I posted a patch from Mail.app, didn't know it did that.
My only other concern is blowing up alpm.h unnecessarily, but I don't see too many other ways of doing this.
Could the enum definition be moved to trans.c and have just the typedef in alpm.h, as is done with __pmdb_t, etc.? I haven't really used enums before so I don't know if it would stay in scope.
Perhaps we can get rid of all the blank comment lines in each desc section? These:
+ *
It appears not. From the doxygen manual:
One could use the \brief command with one of the above comment blocks. This command ends at the end of a paragraph, so the detailed description follows after an empty line. I tried removing the blank line but the detailed description just ended up being on the same line.
-- Sebastian Nowicki
I didn't really know what PM_TRANS_EVT_SCRIPTLET_INFO is for, so I didn't comment it. I glanced at the code but the function in which it is sent is huge and it's late ;). Also when documenting PM_TRANS_EVT_ADD_DONE I looked at the code and it seems like oldpkg is always NULL when the event is sent, so I only documented newpkg. Correct me if I'm wrong on this. The patch is a bit lengthy, but it's very hard to use these events without looking at the code. There was no way of telling what data actually gets passed in without looking at code. I wasn't really sure how to document it, so I just did it how I thought would be logical. ps. I sent the patch from Mail.app so I hope it applies ok. -- Sebastian Nowicki
On Sun, Apr 27, 2008 at 03:19:11AM +0800, Sebastian Nowicki
I didn't really know what PM_TRANS_EVT_SCRIPTLET_INFO is for, so I
the lib can send 3 types of scriptlet events: 2 of them is for "doing something... done." (PM_TRANS_EVT_SCRIPTLET_START, PM_TRANS_EVT_SCRIPTLET_DONE) the third is for everything else, that's PM_TRANS_EVT_SCRIPTLET_INFO. basically if you don't do an "echo START <event desc>" or "echo DONE <ret code>", then the frontend will get the string via a PM_TRANS_EVT_SCRIPTLET_INFO.
I thought that I sent this yesterday, but it seems sendmail wasn't set up properly. I added documentation for PM_TRANS_EVT_SCRIPTLET_INFO, but I'm still not sure if it's correct.
It is hard to decipher what the transaction events actually notify you
of, and what parameters are passed to the callback function, without
looking at the code. This patch adds documentation for the _pmtransevt_t
enum in order to clarify what the event is for and what data is passed
when the callback is called.
Signed-off-by: Sebastian Nowicki
Sebastian Nowicki wrote:
+ + /** Package waas extracted + * + * No parameters are passed + */ PM_TRANS_EVT_EXTRACT_DONE,
s/waas/was
+ + /** Target deltas's integrity will be checked + * + * No parameters are passed + */ PM_TRANS_EVT_DELTA_INTEGRITY_START,
Shouldn't it be deltas' or something? Not even sure :) Except these two typos, it looks alright. I just had the same first impression as Dan, it looks a bit intrusive. Though you already removed one line from the original patch, but too bad we can't remove more. I was thinking, do we really need to specify "No parameters are passed" every time? Maybe having no description could implicitly means that no parameters are passed.
On 27/04/2008, at 5:03 PM, Xavier wrote:
Sebastian Nowicki wrote:
+ + /** Package waas extracted + * + * No parameters are passed + */ PM_TRANS_EVT_EXTRACT_DONE,
s/waas/was
+ + /** Target deltas's integrity will be checked + * + * No parameters are passed + */ PM_TRANS_EVT_DELTA_INTEGRITY_START,
Shouldn't it be deltas' or something? Not even sure :)
Yes, deltas', damn typos :P
Except these two typos, it looks alright. I just had the same first impression as Dan, it looks a bit intrusive. Though you already removed one line from the original patch, but too bad we can't remove more.
I was thinking, do we really need to specify "No parameters are passed" every time? Maybe having no description could implicitly means that no parameters are passed.
For documentation I think explicit is better than implicit. Someone could assume that it's simply not documented. Though I was thinking, maybe "NULL parameters are passed" or something would be better. We could just say "NULL parameters are passed unless specified otherwise" in the documentation of the enum itself. Although the enum's brief doesn't seem to show up anywhere... -- Sebastian Nowicki
This is probably as good as it will get. I fixed the typos, and removed the "No parameters..." lines. The enum documentation was wrong as well, the brief actually added to the group's description, so I fixed that too. The enum description now explicitely states that null parameters are passed unless specified otherwise. Empty lines between comments were removed to slim it down a bit, I don't think it really makes them any harder to read.
It is hard to decipher what the transaction events actually notify you
of, and what parameters are passed to the callback function, without
looking at the code. This patch adds documentation for the _pmtransevt_t
enum in order to clarify what the event is for and what data is passed
when the callback is called.
Signed-off-by: Sebastian Nowicki
On Sun, Apr 27, 2008 at 12:54 PM, Sebastian Nowicki
This is probably as good as it will get. I fixed the typos, and removed the "No parameters..." lines. The enum documentation was wrong as well, the brief actually added to the group's description, so I fixed that too. The enum description now explicitely states that null parameters are passed unless specified otherwise. Empty lines between comments were removed to slim it down a bit, I don't think it really makes them any harder to read.
Good job, I like it!
participants (4)
-
Dan McGee
-
Miklos Vajna
-
Sebastian Nowicki
-
Xavier