On Wed, 2019-09-04 at 17:20 +0200, Ulrich Mueller wrote:
> > > > > > On Wed, 04 Sep 2019, Michał Górny wrote:
> > +# Copyright 2019 Gentoo Authors
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +# @ECLASS: sgml-catalog-r1.eclass
> > +# @MAINTAINER:
> > +# Michał Górny <[email protected]>
> > +# @AUTHOR:
> > +# Michał Górny <[email protected]>
> > +# @BLURB: Functions for installing SGML catalogs
> > +# @DESCRIPTION:
> > +# sgml-catalog-r1 provides postinst/postrm for regenerating
> > +# /etc/sgml/catalog to include all installed catalogs.
> 
> catalog...catalog...catalog...
> 
> Certainly the style could be improved? How about: "This eclass
> regenerates /etc/sgml/catalog in pkg_postinst and pkg_postrm."?
> 
> > +
> > +case "${EAPI:-0}" in
> 
> Quotes aren't necessary here.
> 
> > +   7)
> > +           ;;
> > +   *)
> > +           die "Unsupported EAPI=${EAPI} for ${ECLASS}"
> > +           ;;
> > +esac
> 
> This case statement could be more compact (which would be better
> readable, IMHO).
> 
> > +
> > +EXPORT_FUNCTIONS pkg_postinst pkg_postrm
> > +
> > +if [[ ! ${_SGML_CATALOG_R1} ]]; then
> > +
> > +RDEPEND=">=app-text/sgml-common-0.6.3-r7"
> > +
> > +# @FUNCTION: sgml-catalog-r1_update_catalog
> > +# @DESCRIPTION:
> > +# Regenerate /etc/sgml/catalog to include all installed catalogs.
> > +sgml-catalog-r1_update_catalog() {
> > +   local shopt_save=$(shopt -p nullglob)
> > +   shopt -s nullglob
> > +   local cats=( "${EROOT}"/etc/sgml/*.cat )
> > +   ${shopt_save}
> > +
> > +   if [[ ${#cats[@]} -gt 0 ]]; then
> > +           ebegin "Updating ${EROOT}/etc/sgml/catalog"
> > +           printf 'CATALOG "%s"\n' "${cats[@]}" > "${T}"/catalog &&
> > +           mv "${T}"/catalog "${EROOT}"/etc/sgml/catalog
> > +   else
> > +           ebegin "Removing ${EROOT}/etc/sgml/catalog"
> > +           rm "${EROOT}"/etc/sgml/catalog &&
> > +           { rmdir "${EROOT}"/etc/sgml &>/dev/null || :; }
> > +   fi
> > +   eend "${?}"
> 
> Using one eend for each ebegin would improve readability. Also, quotes
> around $? aren't necessary.
> 
> > +}
> > +
> > +# @FUNCTION: sgml-catalog-r1_update_env
> > +# @DESCRIPTION:
> > +# Regenerate environment variables and copy them to env.d.
> > +sgml-catalog-r1_update_env() {
> > +   # gensgmlenv doesn't support overriding root
> > +   if [[ -z ${ROOT} && -x "${EPREFIX}/usr/bin/gensgmlenv" ]]; then
> > +           ebegin "Regenerating SGML environment variables"
> > +           gensgmlenv &&
> > +           grep -v export "${EPREFIX}/etc/sgml/sgml.env" > 
> > "${T}"/93sgmltools-lite &&
> > +           mv "${T}"/93sgmltools-lite 
> > "${EPREFIX}/etc/env.d/93sgmltools-lite"
> > +           eend "${?}"
> > +   fi
> > +}
> > +
> > +# @FUNCTION: sgml-catalog-r1_pkg_postinst
> > +# @DESCRIPTION:
> > +# Perform catalog post installation tasks.
> 
> Sure, what else would postinst do? :)

Exactly.  I was wondering if it would be good style to just skip
documenting this.

> 
> > +sgml-catalog-r1_pkg_postinst() {
> > +   sgml-catalog-r1_update_catalog
> > +   sgml-catalog-r1_update_env
> > +}
> > +
> > +# @FUNCTION: sgml-catalog-r1_pkg_postrm
> > +# @DESCRIPTION:
> > +# Perform catalog post removal tasks.
> > +sgml-catalog-r1_pkg_postrm() {
> > +   sgml-catalog-r1_update_catalog
> > +   sgml-catalog-r1_update_env
> > +}
> > +
> > +_SGML_CATALOG_R1=1
> > +fi

-- 
Best regards,
Michał Górny

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to