Translate checkArgument messages #4978
Conversation
|
Hi, this is my 2nd PR, just a little less trivial than my first one. I picked it up mainly to learn a bit more about the internals. As I was making changes I realized that this rabbit hole goes deeper than I originally anticipated. On one hand, translating all |
There was a problem hiding this comment.
In general I don't think it's worth moving these strings to resources. Some do have value as they're useful for users, but it's mostly meant as a way to trouble shoot issues together with users, or as a safety check that should never happen but just a good to have.
Adding more strings to resources adds more work for translators for very little benefit. I can still see the benefit of some of these strings so would ack if @ManfredKarrer agrees, as this is a change to core DAO code which can't be merged without his ack as per project guidelines.
| // buyerAmount can be 0 | ||
| if (buyerAmount.isPositive()) { | ||
| Preconditions.checkArgument(Restrictions.isAboveDust(buyerAmount), | ||
| "The buyerAmount is too low (dust limit)."); | ||
| Res.get("validation.amountBelowDust.shortVar", "buyerAmount")); |
There was a problem hiding this comment.
This kind of argument defeats the aim of making the strings translatable. In this case it's better to just make two strings, or reuse validation.amountBelowDust.short
There was a problem hiding this comment.
NACK
I don't think we should start translating those type of strings. They are mostly in place for catching edge case bugs and for debugging. In case such an error occurs the user can send a screenshot of the error popup to developers. It is not intended as error messages the user can deal with, so translation does not add value here.
It would also add more effort in dev process and for translators.
I appreciate your intentions and effort but I fear that PR is going in the wrong direction. Best to search the GH issue list for "best first issues" or the like. There are plenty of issues and TODOs open specially in the UI side which would add lot of value to Bisq.
|
Understood. I did pick an issue tagged with "good first issue", sadly it seems that the issue wasn't very well defined. I agree that most instances of |
|
@Jakub-CZ Yes, not your fault but Bisq's lack of maintaining the GH issues better.... too much work for too few devs ;-) |
|
@Jakub-CZ Sorry about that! This was marked incorrectly as a |
Fixes #2619
Messages in some checkArgument calls are replaced with translatable strings.
There are many more instances, incl. ones in calls of similar functions (notEmpty, notBlank, notNull). However, most seem to be unreachable by the users; the GUI prevents the conditions from ever happening. Many messages are not even intended to be seen by users; they include things like private method names and local variable names.
In fact, I wasn't able to reproduce even some of those few messages that are updated by this PR.