Skip to content

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

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

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

Conversation

@fanquake

@fanquake fanquake commented May 31, 2021

Copy link
Copy Markdown
Member

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

@hebasto

hebasto commented May 31, 2021

Copy link
Copy Markdown
Member

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

@maflcko maflcko left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread src/qt/guiutil.cpp Outdated
Comment thread src/qt/transactiontablemodel.cpp Outdated
Comment thread src/qt/transactiontablemodel.cpp Outdated
hebasto
hebasto reviewed May 31, 2021
View reviewed changes
Comment thread src/qt/transactionfilterproxy.cpp Outdated
@promag

promag commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

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
Comment thread src/qt/transactionfilterproxy.cpp Outdated
maflcko
maflcko approved these changes Jun 3, 2021
View reviewed changes

@maflcko maflcko left a comment

Copy link
Copy Markdown
Contributor

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
Comment thread src/qt/transactiontablemodel.cpp Outdated
Comment thread src/qt/transactiontablemodel.cpp Outdated
@maflcko

maflcko commented Jun 7, 2021

Copy link
Copy Markdown
Contributor

cr ACK 4f5e9fe

@fanquake

fanquake commented Jun 7, 2021

Copy link
Copy Markdown
Member Author

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

hebasto
hebasto approved these changes Jun 7, 2021
View reviewed changes

@hebasto hebasto left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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

@Talkless Talkless left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Concept ACK

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

@Talkless Talkless Jun 17, 2021

Copy link
Copy Markdown

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() ?

Comment thread src/qt/transactionfilterproxy.cpp Outdated
@maflcko

maflcko commented Aug 23, 2021

Copy link
Copy Markdown
Contributor

cr ACK bfcde3e

maflcko
maflcko reviewed Aug 23, 2021
View reviewed changes
Comment thread src/qt/recentrequeststablemodel.h
hebasto
hebasto reviewed Aug 23, 2021
View reviewed changes

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approach ACK bfcde3e.

Comment thread src/qt/transactionfilterproxy.cpp Outdated
@hebasto

hebasto commented Aug 24, 2021

Copy link
Copy Markdown
Member 
./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
Comment thread src/qt/recentrequeststablemodel.h Outdated
hebasto
hebasto approved these changes Aug 24, 2021
View reviewed changes

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

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
MarcoFalke jagdeep sidhu
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