doc/plugins/contrib/listsubscribe/discussion.mdwn

   1 ## Code review
   2
   3 ### Main things
   4
   5 [[!format diff """
   6 +<form method="get" action="<TMPL_VAR LISTSUBSCRIBEACTION>" id="listsubscribeform">
   7 """]]
   8
   9 This should have `ESCAPE=HTML` (see [[!cpan HTML::Template]]). The other TMPL_VARs probably
  10 should too.
  11
  12 The method should be `post`, and the CGI should ideally not respond to `GET`s (because sending
  13 email isn't idempotent).
  14
  15 ### Minor things
  16
  17 It would be good to have the documentation specify exactly what "API" it expects from the
  18 mailing list: in this case it seems to be an address to which you can send a blank message
  19 with the desired subscription address in the `From:` header. I believe that works for most
  20 but not all mailing list managers (hopefully the ones where you're meant to mail the posting
  21 address with "subscribe" in the body have died out by now).
  22
  23 [[!format diff """
  24 +<h4>Join <TMPL_VAR LISTSUBSCRIBELISTNAME></h4>
  25 """]]
  26
  27 Might be better to have a separate parameter for the human-readable name?
  28
  29 [[!format diff """
  30 +       my $list_subscribe_form = template('listsubscribeform.tmpl');
  31 """]]
  32
  33 I wonder whether this would benefit from an optional `template` parameter, with this as default?
  34
  35 ```
  36 listsubscribe:
  37   'my supercool mailing list': supercool-subscribe@neato.great
  38 ```
  39
  40 This won't be available to [[plugins/websetup]], which doesn't understand hashes/dicts/maps.
  41 If it was a list of flat strings with a syntactic structure, like the language list for `po`,
  42 then it would be. Sorry, I can't think of a particularly good syntax...
  43
  44 --[[smcv]]