David W. Van Couvering wrote:
Hi, Andreas. Thanks for reviewing this.
I was trying to give the other-side-of-pond folks a day to review it by
posting the patch and request for review last night.
Well, thanks for contributing this important piece of work.
I think I missed the first request for review. I guess it only got my
full attention when you posted the deadline.
How much time do you need?
I cannot review it more tonight - I am fine with your responses to my
comments.
I will look at it tomorrow, and let you know if I have found any issues,
regardless if you decide to check-in or not.
Andreas
Let me respond to your comments below.
Andreas Korneliussen wrote:
David W. Van Couvering wrote:
Hi, all. This patch is blocking further i18n work for me. I also
don't want to have to do any merges and re-run derbyall.
If you could get any comments in by noon, that would be appreciated,
otherwise I will go ahead and check in and we can fix anything
post-checkin.
This was quite a short deadline - if you want comments from the other
side of the world, it would be wise to extend the deadline.
I have only looked briefly at the diff file. I think that the changes
in NetConnection40 and NetResultSet40 are not necessary (it appears
the changes only fix code indentation), so to avoid getting other
developers into merge conflicts, you could probably revert those changes.
Another issue, is this error message:
ERROR 08004: Connection authorization failure occurred. Reason:
userid invalid.
I think, for security policy reasons, the message should not reveal
that userid is invalid.
Hm, I agree. That said, "invalid userid" and "invalid password" are
both standard DRDA values for the SECCHKCD parameter. I guess I can say
"invalid userid or password" or "invalid credentials" for both of them,
that's a pretty common approach.
Also, shouldn't it read "authentication failed", instead of
"authorization failure occured" ?
Yes, I agree, I can change that.
(note: the patch did not really introduce this last issue, only fixed
the message to have messageid as well)
-- Andreas
Thanks,
David
David W. Van Couvering wrote:
This patch is the first patch internationalizing strings in the
network part of the network client. I thought it would be good to
get the eyes of some of the folks working in this area on this, to
make sure it makes sense to you.
http://issues.apache.org/jira/browse/DERBY-846?page=all
Thanks,
David
