Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date #137

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Nov 27, 2020

As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.

Required for bitcoin/bitcoin#20182.

Details in Qt docs:

@promag
Copy link
Contributor

promag commented Nov 27, 2020

Was this identified with the latest 5.15.2?

@hebasto
Copy link
Member Author

hebasto commented Nov 27, 2020

Was this identified with the latest 5.15.2?

5.15.2 was released on 2020-11-20, but the recent Qt the Homebrew provides still 5.15.1.

Warnings are fired on 5.15.1:

Did not test on 5.15.2.

UPDATE: Looking into code it seems I just missed those warnings in #46 🤷‍♂️

@promag
Copy link
Contributor

promag commented Nov 27, 2020

UPDATE: Looking into code it seems I just missed those warnings in #46 🤷‍♂️

Yeah, easy to miss.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1

  • Compiled both Master and PR with make -j{n} |& tee compile.txt, then grepped this file with grep -r "deprecated"
  • Can confirm that master raises these deprecation warnings and the PR fixes them

I also compiled with Qt 5.15.2 on Linux and can confirm that master still raises, and PR fixes. Furthermore, there are no new deprecated warnings.

Copy link
Contributor

@jonasschnelli jonasschnelli left a comment

Choose a reason for hiding this comment

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

Tested ACK 86b1ab6

@jonasschnelli jonasschnelli merged commit f2a673f into bitcoin-core:master Dec 1, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 1, 2020
…cale{Short,Long}Date

86b1ab6 refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date (Hennadii Stepanov)

Pull request description:

  As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.

  Required for bitcoin#20182.

  Details in Qt docs:
  - https://doc.qt.io/qt-5/qdatetime.html#toString-1
  - https://doc.qt.io/qt-5/qdate.html#toString-1

ACKs for top commit:
  jarolrod:
    Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1
  jonasschnelli:
    Tested ACK 86b1ab6

Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
@hebasto hebasto deleted the 201127-locale branch December 1, 2020 20:15
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants