Make initTranslations a member function of BitcoinApplication class #423

pull jarolrod wants to merge 1 commits into bitcoin-core:master from jarolrod:move-init-translations changing 2 files +42 −40

jarolrod commented at 2:45 AM on September 15, 2021: member

Moves the initTranslations function from a static function to a member function of the BitcoinApplication class. This is for correctness and to clarify that the QApplication being worked on is our BitcoinApplication object.

qt: make initTranslations a member function of BitcoinApplication class

Moves the initTranslations function from a static function to a member
function of the BitcoinApplication class. This is for correctness
and to clarify that the QApplication being worked on is our
BitcoinApplication object.

2b6c529773

jarolrod renamed this:
~~qt: make initTranslations a member function of BitcoinApplication class~~
Make initTranslations a member function of BitcoinApplication class
on Sep 15, 2021
jarolrod added the label Refactoring on Sep 15, 2021
shaavan commented at 11:33 AM on September 15, 2021: contributor

Concept ACK

removeTranslator and installTranlator function removes/installs translation files to “this” application. (Source removeTranslator, installTranslator ) It makes sense for a function that deals with these two functions to be part of the BitcoinApplication class, then be a standalone static function.
hebasto commented at 2:26 PM on September 15, 2021: member

This is for correctness...

QCoreApplication::installTranslator is a static function member of the QCoreApplication class.

And I cannot get the point of moving it into the non-static function member of the BitcoinApplication class when we actually need no access to any data member of the latter.
jarolrod commented at 3:42 PM on September 15, 2021: member

the mentioned installTranslator is a function that we use within our initTranslator function. I think this makes things clearer, but this is extremely low-priority.

Feel free to close if not wanted 🥃
DrahtBot commented at 9:49 PM on June 12, 2022: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #623 (Getting ready to Qt 6 (9/n). Apply Qt 6 specific changes by hebasto)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
jarolrod closed this on Jun 22, 2022

in src/qt/bitcoin.h:52 in 2b6c529773

  47 | @@ -47,6 +48,8 @@ class BitcoinApplication: public QApplication
  48 |      void createOptionsModel(bool resetSettings);
  49 |      /// Initialize prune setting
  50 |      void InitPruneSetting(int64_t prune_MiB);
  51 | +    /** Set up translations */
  52 | +    void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTranslator, QTranslator &translatorBase, QTranslator &translator);

laanwj commented at 8:39 AM on June 22, 2022:

What about making it a static member function? I slightly like the idea of grouping it that way, but making it a normal member function is not needed) (Sure, closing is also fine with me)

bitcoin-core locked this on Jun 22, 2023

Contributors

Labels

Make initTranslations a member function of BitcoinApplication class #423

Conflicts