[pacman-dev] [PATCH 1/3] alpm.h: add Doxygen-style doc to package-related functions.

[Dan McGee]

On Tue, Mar 1, 2011 at 2:16 PM, Rémy Oudompheng <remy at archlinux.org> wrote:
> ---
Wow. First off this is awesome, thanks.

>  lib/libalpm/alpm.h |  167 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 167 insertions(+), 0 deletions(-)
>
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 0f3b716..7fa27d8 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -199,36 +199,203 @@ int alpm_db_set_pkgreason(pmdb_t *db, const char *name, pmpkgreason_t reason);
>
>  /* Info parameters */
>
> +/** Create a package from a file.
> + * If full is false, the archive is read only until all necessary
> + * metadata is found. If it is true, the entire archive is read, which
> + * serves as a verfication of integrity and the filelist can be created.
> + * @param filename location of the package tarball
> + * @param full whether to stop the load after metadata is read or continue
> + *             through the full archive
> + * @param pkg address of the package pointer
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> + */
>  int alpm_pkg_load(const char *filename, int full, pmpkg_t **pkg);
It looks like you copied this in here, but didn't remove it from the
other location.

I think we need to have it in one place only if at all possible, or we
will encounter the situation where one version gets updated and the
other does not. Doxygen correctly pulls it from either location (see
alpm_initialize, etc.), but it is definitely nice to have in the
header, so let's kill it from be_package.c in this case.

We will come across it moving on, but we should also pick a format we
plan to follow, as I've seen several:
1) Put first line on /** line, everything else follows after a
newline. This is the most compact.
2) Same, but newline and empty line after first line.
3) Use @brief, then a empty line, then remaining text. Things also
seem to have spaces between every part (brief, summary, params).
4) Compact but using @brief.

I don't care- as I said, I'm letting you call the shots, but we should
try and pick one and stick with it and eventually retrofit everything
to work this way.

> +
> +/** Free a package.
> + * @param pkg package pointer to free
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> + */
>  int alpm_pkg_free(pmpkg_t *pkg);
> +
> +/** Check the integrity (with md5) of a package from the sync cache.
> + * @param pkg package pointer
> + * @return 0 on success, -1 on error (pm_errno is set accordingly)
> + */
>  int alpm_pkg_checkmd5sum(pmpkg_t *pkg);
If you're going to stop documenting the next few, I'd at least add a
newline here so it doesn't look like you're documenting the whole
group.

>  char *alpm_fetch_pkgurl(const char *url);
>  int alpm_pkg_vercmp(const char *a, const char *b);
>  alpm_list_t *alpm_pkg_compute_requiredby(pmpkg_t *pkg);
>
> +/**
> + * Package property accessors
> + *
> + * These functions return pointers to internal structures allocated by
> + * libalpm. They should not be freed.
or modified in any way unless otherwise documented.

I say this because you should stick the above requiredby function in
this group, and document that the contents MUST be freed by the caller
as it is computed, not "gotten". (A list of strings apparently...not
sure why it isn't pmpkg_t).

> + */
> +
> +/**
> + * @brief Gets the name of the file from which the package was loaded.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_filename(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package name.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_name(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package version as a string.
This includes all available epoch, version, and pkgrel components. Use
####(however you ref a func) alpm_pkg_vercmp() to compare version
strings if necessary.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_version(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package description.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_desc(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package URL.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_url(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the build timestamp of the package.
> + * @param pkg a pointer to package
> + * @return the timestamp of the build time
> + */
>  time_t alpm_pkg_get_builddate(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the install timestamp of the package.
> + * @param pkg a pointer to package
> + * @return the timestamp of the install time
> + */
>  time_t alpm_pkg_get_installdate(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the packager's name.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_packager(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package md5sum as a string of hexadecimal digits.
uppercase or lowercase? I believe it is lower.

> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_md5sum(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the architecture for which the package was built.
> + * @param pkg a pointer to package
> + * @return a reference to an internal string
> + */
>  const char *alpm_pkg_get_arch(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the size of the package
period at end
> + * @param pkg a pointer to package
> + * @return the size of the package in bytes.
> + */
>  off_t alpm_pkg_get_size(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the installed size of the package
period at end
> + * @param pkg a pointer to package
> + * @return the total size of files installed by the package.
> + */
>  off_t alpm_pkg_get_isize(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the package installation reason.
> + * @param pkg a pointer to package
> + * @return a pmpkgreason_t member giving the install reason.
Relisting the non-list return type here is a bit silly as that ends up
in any doxygen output I've seen anyway.
> + */
>  pmpkgreason_t alpm_pkg_get_reason(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package licenses.
> + * @param pkg a pointer to package
> + * @return a pointer to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_licenses(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package groups.
> + * @param pkg a pointer to package
> + * @return a pointer to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_groups(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package dependencies as pmdepend_t.
> + * @param pkg a pointer to package
> + * @return a reference to an internal list of pmdepend_t structures.
> + */
>  alpm_list_t *alpm_pkg_get_depends(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package optional dependencies.
> + * @param pkg a pointer to package
> + * @return a reference to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_optdepends(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package names conflicting with pkg.
> + * @param pkg a pointer to package
> + * @return a reference to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_conflicts(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of package names provided by pkg.
> + * @param pkg a pointer to package
> + * @return a reference to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_provides(pmpkg_t *pkg);
>  alpm_list_t *alpm_pkg_get_deltas(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of packages to be replaced by pkg.
> + * @param pkg a pointer to package
> + * @return a reference to an internal list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_replaces(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of files installed by pkg.

The list will contain filename strings relative to the install root,
so no leading slashes will be provided.

> + * @param pkg a pointer to package
> + * @return a reference to an internal list of filenames.
list of strings? as you do everywhere else.
> + */
>  alpm_list_t *alpm_pkg_get_files(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the list of files backed up when installing pkg.

This one is actually a mess.

The strings are formatted as such:
filename<tab char>md5sum

Where md5sum is the checksum of the stock file out of the package. Ew.

> + * @param pkg a pointer to package
> + * @return a reference to an internal list of filenames.
list of strings.
> + */
>  alpm_list_t *alpm_pkg_get_backup(pmpkg_t *pkg);
> +
> +/**
> + * @brief Returns the pmdb_t containing pkg
> + *
> + * Returns a pointer to the pmdb_t structure the package is
> + * originating from, or NULL is the package was loaded from a file.
> + * @param pkg a pointer to package
> + * @return a pointer to the DB containing pkg, or NULL.
> + */
>  pmdb_t *alpm_pkg_get_db(pmpkg_t *pkg);
Newline here if you aren't going to doc these.
>  void *alpm_pkg_changelog_open(pmpkg_t *pkg);
>  size_t alpm_pkg_changelog_read(void *ptr, size_t size,
> --
> 1.7.4.1