[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [O] org-bbdb-anniversaries-future

From: Nicolas Goaziou
Subject: Re: [O] org-bbdb-anniversaries-future
Date: Sun, 05 Mar 2017 18:28:10 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)


Michael Welle <address@hidden> writes:

> Maybe it's a tiny change, if we strip the comments ;). It's not too
> sophisticated, everyone could have done it.

I definitely think it _is_ a tiny change.

Some comments follow.

> +(defun org-bbdb-anniversary-description (agenda-date anniv-date)
> +  "Return a string used to modify an agenda anniversary entry. The 

The first line needs to be a sentence on its own. IOW, the last "The"
belong to the next line.
> +   calculation of the string is based on the difference between

Indentation in this line and all subsequent ones needs to be removed.

> +   the anniversary date and the date on which the entry appears
> +   in the agenda. This makes it possible to have different entries

Docstrings follow US English conventions, i.e., two spaces are required
after a full stop.

> +   for the same event depending on if it occurs in the next few days
> +   or far away in the future."

You also need to document what are the arguments (type).  They are
written up-cased in the docstring.

> +  (let ((delta (- (calendar-absolute-from-gregorian anniv-date)
> +                  (calendar-absolute-from-gregorian agenda-date))))
> +
> +    (cond
> +     ((= delta 0) " -- today\\&")
> +     ((= delta 1) " -- tomorrow\\&")
> +     ((< delta 7) (format " -- in %d days\\&" delta))
> +     ((format " -- %d-%02d-%02d\\&" (third anniv-date) (first anniv-date) 
> (second anniv-date))))))

You need to used "cl-" prefix, e.g., `cl-third'. OTOH, I'd rather use
pattern matching here, for clarity:

  (pcase-let ((`(,month ,day ,year) anniv-date))
    (format " -- %s-%02%d-%02d\\&" year month day))

Also, could you provide a commit message?

Thank you.


Nicolas Goaziou

reply via email to

[Prev in Thread] Current Thread [Next in Thread]