Skip to content

replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch#349

Merged
maflcko merged 2 commits intobitcoin-core:masterfrom
fanquake:qt_drop_from_time_t
Aug 24, 2021
Merged

replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch#349
maflcko merged 2 commits intobitcoin-core:masterfrom
fanquake:qt_drop_from_time_t

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented May 31, 2021

QDateTime::fromTime_t has been obsoleted in favour of QDateTime::fromSecsSinceEpoch, which is available from Qt 5.8+.

@hebasto
Copy link
Member

hebasto commented May 31, 2021

Concept ACK.

Interesting that there are no [-Wdeprecated-declarations] warnings in the build log (Fedora 34 + Qt 5.15.2).

maflcko
maflcko reviewed May 31, 2021
View reviewed changes
Copy link
Contributor

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

You are passing the wrong type. Otherwise Concept ACK

hebasto
hebasto reviewed May 31, 2021
View reviewed changes
@promag
Copy link
Contributor

promag commented Jun 2, 2021

Concept ACK. No longer scripted-diff.

@fanquake fanquake changed the title scripted-diff: replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch Jun 3, 2021
maflcko
maflcko reviewed Jun 3, 2021
View reviewed changes
maflcko
maflcko approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

ACK

hebasto
hebasto requested changes Jun 6, 2021
View reviewed changes
@maflcko
Copy link
Contributor

maflcko commented Jun 7, 2021

cr ACK 4f5e9fe

@fanquake
Copy link
Member Author

fanquake commented Jun 7, 2021

Added one additional commit to replace QDateTime::toTime_t with QDateTime::toSecsSinceEpoch.

hebasto
hebasto approved these changes Jun 7, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK bd533e7, (will look into CI failures)

$ git grep fromTime_t | wc -l
0
$ git grep toTime_t | wc -l
0

Maybe update the OP to mention the second commit changes?

Talkless
Talkless reviewed Jun 17, 2021
View reviewed changes
Copy link

@Talkless Talkless 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

src/qt/recentrequeststablemodel.cpp
{
case RecentRequestsTableModel::Date:
return pLeft->date.toTime_t() < pRight->date.toTime_t();
return pLeft->date.toSecsSinceEpoch() < pRight->date.toSecsSinceEpoch();
Copy link

@Talkless Talkless Jun 17, 2021

Choose a reason for hiding this comment

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

Out of scope, but I wonder why bool QDateTime::operator< is not used directly, without toTimeT()/toSecsSinceEpoch() ?

@maflcko
Copy link
Contributor

maflcko commented Aug 23, 2021

cr ACK bfcde3e

maflcko
maflcko reviewed Aug 23, 2021
View reviewed changes
hebasto
hebasto reviewed Aug 23, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Approach ACK bfcde3e.

@hebasto
Copy link
Member

hebasto commented Aug 24, 2021 

./serialize.h:676:6: error: member reference base type 'long long' is not a structure or union
    a.Unserialize(is);
    ~^~~~~~~~~~~~

maflcko
maflcko reviewed Aug 24, 2021
View reviewed changes
hebasto
hebasto approved these changes Aug 24, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 3ae503c

@maflcko maflcko merged commit dd455ec into bitcoin-core:master Aug 24, 2021
@fanquake fanquake deleted the qt_drop_from_time_t branch August 24, 2021 08:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 24, 2021
Merge bitcoin-core/gui#349: replace QDateTime::fromTime_t with QDateT…
f993eaf
@barton2526 barton2526 mentioned this pull request Nov 18, 2021
Merged
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

+3 more reviewers

@Talkless Talkless Talkless left review comments

@promag promag promag left review comments

@maflcko maflcko maflcko approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fanquake @hebasto @promag @maflcko @Talkless @DrahtBot