| Message ID | 20190809163702.12605-1-rustand.lars@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | notify.py: Use a/an correctly when sending request notifications | expand |
On 8/9/19 12:37 PM, Lars Rustand wrote: > Will no longer send notifications about "a orphan request", but determine > whether to use a/an based on the first character of the request type. Thanks, looks like a reasonable change. See below for implementation nitpicks. > Signed-off-by: Lars Rustand <rustand.lars@gmail.com> > --- > aurweb/scripts/notify.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py > index d975086..ace6614 100755 > --- a/aurweb/scripts/notify.py > +++ b/aurweb/scripts/notify.py > @@ -414,8 +414,9 @@ class RequestOpenNotification(Notification): > (self._user, self._pkgbase, self._merge_into) > body += '\n\n' + self._text > else: > - body = '%s [1] filed a %s request for %s [2]:' % \ > - (self._user, self._reqtype, self._pkgbase) > + an = ["a","an"][self._reqtype[0] in "aeiou"] Using ['a', 'an'][True] as a list index by relying on it equaling the `1` offset due to implicitly `int(True)` equaling `1` feels pretty unreadable and I don't think I've ever seen that pattern, what about instead using a standard ternary operator: an = 'an' if self._reqtype[0] in 'aeiou' else 'a' Also: the aurweb codebase generally uses single quotes (and does, for the surrounding lines), so I think we should stick to that. > + body = '%s [1] filed %s %s request for %s [2]:' % \ > + (self._user, an, self._reqtype, self._pkgbase) > body += '\n\n' + self._text > return body > >
On 8/9/19 2:08 PM, Lars Rustand wrote: > Argh, gmail replies to the wrong address. > > On Fri, Aug 9, 2019, 20:06 Lars Rustand <rustand.lars@gmail.com> wrote: > >> Thank you for the feedback, I totally agree with your remarks. I might be >> a little too fond of codegolfing, that's where I got the idea to use True >> as an index. >> I posted an updated patch where I added your changes. No problem. ;) Thanks. As an aside, I have now seen int(True)-indexed lists used "in the wild"! A clever fox saw what I wrote and PM'ed me a Fizzbuzz implementation which relied on that.
On Fri, Aug 09, 2019 at 02:59:13PM -0400, Eli Schwartz wrote: > On 8/9/19 2:08 PM, Lars Rustand wrote: > > Argh, gmail replies to the wrong address. > > > > On Fri, Aug 9, 2019, 20:06 Lars Rustand <rustand.lars@gmail.com> wrote: > > > >> Thank you for the feedback, I totally agree with your remarks. I might be > >> a little too fond of codegolfing, that's where I got the idea to use True > >> as an index. > >> I posted an updated patch where I added your changes. > > No problem. ;) Thanks. > > As an aside, I have now seen int(True)-indexed lists used "in the wild"! > A clever fox saw what I wrote and PM'ed me a Fizzbuzz implementation > which relied on that. Pft. Implying it's not public on github! https://gist.github.com/Foxboron/6643154
diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index d975086..ace6614 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -414,8 +414,9 @@ class RequestOpenNotification(Notification): (self._user, self._pkgbase, self._merge_into) body += '\n\n' + self._text else: - body = '%s [1] filed a %s request for %s [2]:' % \ - (self._user, self._reqtype, self._pkgbase) + an = ["a","an"][self._reqtype[0] in "aeiou"] + body = '%s [1] filed %s %s request for %s [2]:' % \ + (self._user, an, self._reqtype, self._pkgbase) body += '\n\n' + self._text return body
Will no longer send notifications about "a orphan request", but determine whether to use a/an based on the first character of the request type. Signed-off-by: Lars Rustand <rustand.lars@gmail.com> --- aurweb/scripts/notify.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)