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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Aug 24, 2021
Merged

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

merged 2 commits into from
Aug 24, 2021

Conversation

fanquake
Copy link
Member

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

src/qt/guiutil.cpp Outdated Show resolved Hide resolved
src/qt/transactiontablemodel.cpp Outdated Show resolved Hide resolved
src/qt/transactiontablemodel.cpp Outdated Show resolved Hide resolved
@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 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
src/qt/transactiontablemodel.cpp Outdated Show resolved Hide resolved
src/qt/transactiontablemodel.cpp Outdated Show resolved Hide resolved
@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
@@ -234,7 +234,7 @@ bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, cons
switch(column)
{
case RecentRequestsTableModel::Date:
return pLeft->date.toTime_t() < pRight->date.toTime_t();
return pLeft->date.toSecsSinceEpoch() < pRight->date.toSecsSinceEpoch();
Copy link

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

src/qt/transactionfilterproxy.cpp Outdated Show resolved Hide resolved
@maflcko
Copy link
Contributor

maflcko commented Aug 23, 2021

cr ACK bfcde3e

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.

src/qt/transactionfilterproxy.cpp Outdated Show resolved Hide resolved
@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);
    ~^~~~~~~~~~~~

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

@maflcko maflcko maflcko approved these changes

@promag promag promag left review comments

@Talkless Talkless Talkless left review comments

@hebasto hebasto hebasto approved these changes

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