Eric wrote:
OAuth is a very modern thing to bring into nmh.
Wow, cool, thanks!
I enable a libcurl dependency only when configured with
--with-oauth which is off by default. But practically no one has
jsmn installed, so I'm suggesting we include it directly.
I think that might be unprecedented for nmh. But I hope it's not
too controversial.
I think that's fine.
- mhlogin name / flag names
Naming is hard :). I picked this on the theory that it's not
terribly confusing as is, and if there were to be some other
kind of system users might need to login to, expanding mhlogin
to have more than just -oauth would make sense.
I like it.
- Repeating -user for each command is possibly odd. Maybe put
-user on mhlogin and save it in the cred file. Arguably easier
-for the user this way, arguably not. Changing it would
-complicate the code slightly. I don't really care either way.
Would it be easier with multiple accounts to use -user everywhere?
I'm thinking shell aliases/scripts. It could also be done with
the cred file and $MH, but I think that's marginally messier.
- I have a lot of test cases in only a few broadly categorized
test scripts, and they print descriptions as they go so it's
easy to see what broke. This messes up the test suite output.
Does this make sense, should I change this only to print only
if some environment variable is set,
Or just if something goes wrong?
test-mhlogin failed for me because each line of my actual output
ends with a Ctrl-M. On Linux, so I don't know why.
Any idea with test-inc and test-msgcheck would fail with:
'inc: POP server does not support SASL'
'msgchk: POP server does not support SASL'
I did build with sasl, of course, and confirmed with mhparam.
Of course, I welcome criticism on all other aspects too: API,
documentation, organization, whatever.
I didn't dig in to any of that, but just noticed when applying the
patch:
warning: squelched 5 whitespace errors
warning: 10 lines add whitespace errors.
You chose not to use Cyrus SASL for XOAUTH2. I wouldn't
hesitate to use it: nmh already can be configured Cyrus SASL
and some of us do use it. It is configured in by the Fedora
package.
To do:
* add "mhparam oauth" support
* add libcurl and libcurl-devel (on Linux) to MACHINES
* add reference to jsmn LICENSE to COPYRIGHT
David
_______________________________________________
Nmh-workers mailing list
Nmh-workers(_at_)nongnu(_dot_)org
https://lists.nongnu.org/mailman/listinfo/nmh-workers