RocketChat
Replacing magic numbers in the code by settings
There are a few "magic numbers" in the code, such as in https://github.com/RocketChat/Rocket.Chat/blob/a1c41a16c550968761dd0be84673318e7bc63b10/app/lib/server/functions/saveUser.js#L231
In this example, is there a reason why 260 has not been replaced by something like settings.get('Accounts_MaxBioLength')? (With the corresponding setting added to app/lib/server/startup/settings.ts).
Hi, I have made some changes in the code according to the request. Can someone please do my PR review? I am just a beginner in making PR's.
Hello! So, as I commented here, generally adding settings for this kind of stuff isn't a good idea. It adds little value and unnecessary complexity for maintaining it afterwards.
A better option for it would be to add a const with a descriptive name so we know what the number means.
Being able to change these settings is quite valuable for some use cases. I'm using Rocket.chat for a community where people don't necessarily know each other and we definitely need the Bio field to be longer.
Hey, I had not seen your last comment. This issue was closed automatically due to the merge of the foremetioned PR.
I think we could consider just making the field bigger by default then. I really see no problem in having a few more characters in this field. What do you think?
Hey guys, so the solution in this case is to increase the number of characters in MAX_BIO_LENGTH ?
If this is the solution, what's the ideal amount of characters?
Hey, I had not seen your last comment. This issue was closed automatically due to the merge of the foremetioned PR.
I think we could consider just making the field bigger by default then. I really see no problem in having a few more characters in this field. What do you think?
Hi, I am a beginner and if this issue is still open I would like to work on it?
Hey!! I want to work on this issue. I'm new here so I'm really excited to start contributing. Please assign it to me.
Hi @dudanogueira! I think adding a few consts instead of adding these values in the settings.ts file is a more elegant solution. Can I go ahead and work on this approach?
