Skip to content

Conversation

@jarolrod
Copy link
Contributor

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.

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.
@jarolrod jarolrod changed the title qt: make initTranslations a member function of BitcoinApplication class Make initTranslations a member function of BitcoinApplication class Sep 15, 2021
Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

hebasto commented Sep 15, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor

DrahtBot commented Jun 12, 2022

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.

/// Initialize prune setting
void InitPruneSetting(int64_t prune_MiB);
/** Set up translations */
void initTranslations(QTranslator &qtTranslatorBase, QTranslator &qtTranslator, QTranslator &translatorBase, QTranslator &translator);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 bitcoin-core locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants