Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard

Cleaning: replacing `format` by formatted strings

Open Arthur-Milchior opened this issue 2 years ago • 1 comments

This is part of Kotlin clean-up #10489, low priority, a way to slightly improve code quality and make easy changes.

There are currently 37 occurrences of String.format in Kotlin followed by a String literal. In Kotlin, you can generally use Template stings, which I find more readable. It'd be nice to replace them. In short, if you don't know string literals, you just have to use $varName or ${expression}. So for example you'd use "$appName $pkgVersionName" instead of "String.format("%s v%s", appName, pkgVersionName)".

If it's not too hard, I appreciate if you can do one change by pull request; unless multiple changes are very strongly related. This way, those PR can be merged very quickly; and if there is an issue, I don't have to delay all other changes.

You don't have to ask permission to work on this. Just look at the answer below to see links to PR already done. I'll try to merge them quickly. Hopefully, most change should be done in less than a minute, so the risk of wasted work is really small

Arthur-Milchior avatar May 29 '22 17:05 Arthur-Milchior

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar Jul 28 '22 17:07 github-actions[bot]

Can I work on this?

sparkyniner avatar Aug 24 '22 11:08 sparkyniner

@sparkyniner please note this in the description:


If it's not too hard, I appreciate if you can do one change by pull request; unless multiple changes are very strongly related. This way, those PR can be merged very quickly; and if there is an issue, I don't have to delay all other changes.

You don't have to ask permission to work on this. Just look at the answer below to see links to PR already done. I'll try to merge them quickly. Hopefully, most change should be done in less than a minute, so the risk of wasted work is really small


emphasis added

mikehardy avatar Aug 24 '22 15:08 mikehardy

Was busy with some other PR, will work on this asap.

sparkyniner avatar Sep 02 '22 12:09 sparkyniner

Hello, I have issued a PR and changed one instance in one file as requested, if this is ok I can change the other 36 instances.

sparkyniner avatar Sep 07 '22 04:09 sparkyniner

I just edited the issue message to be explicit about locales. Essentially that this issue is mostly about format that are applied directly to a format string, and is not about ones where locale are explicit.

Arthur-Milchior avatar Sep 07 '22 14:09 Arthur-Milchior

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar Dec 26 '22 10:12 github-actions[bot]

IMO, this should be closed because:

  1. It is open-ended
  2. The added value is infimal
  3. It becomes harder with each removal of String.format
  4. Our human resources (contributors and reviewers' time) are quite limited, so it's better to spend it on other issues

I'll give at least a week for anyone else to disagree. If there are no complaints, I'll be closing this

BrayanDSO avatar Feb 04 '23 19:02 BrayanDSO

is somebody working on this?

itsAfnanAlam avatar Feb 10 '23 02:02 itsAfnanAlam

is somebody working on this?

This issue is going to be closed, so there shouldn't be anyone working on this

BrayanDSO avatar Feb 10 '23 16:02 BrayanDSO