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

pull fanquake wants to merge 2 commits into bitcoin-core:master from fanquake:qt_drop_from_time_t changing 8 files +15 −11
  1. fanquake commented at 5:02 am on May 31, 2021: member
    QDateTime::fromTime_t has been obsoleted in favour of QDateTime::fromSecsSinceEpoch, which is available from Qt 5.8+.
  2. hebasto commented at 8:02 am on May 31, 2021: member

    Concept ACK.

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

  3. in src/qt/guiutil.cpp:84 in 01ba23e48f outdated 
    80@@ -81,7 +81,7 @@ QString dateTimeStr(const QDateTime &date)
81 
82 QString dateTimeStr(qint64 nTime)
83 {
84-    return dateTimeStr(QDateTime::fromTime_t((qint32)nTime));
85+    return dateTimeStr(QDateTime::fromSecsSinceEpoch((qint32)nTime));
    MarcoFalke commented at 8:11 am on May 31, 2021: 
    0    return dateTimeStr(QDateTime::fromSecsSinceEpoch(nTime));
  4. in src/qt/transactiontablemodel.cpp:613 in 01ba23e48f outdated 
    609@@ -610,7 +610,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
610     case TypeRole:
611         return rec->type;
612     case DateRole:
613-        return QDateTime::fromTime_t(static_cast<uint>(rec->time));
614+        return QDateTime::fromSecsSinceEpoch(static_cast<uint>(rec->time));
    MarcoFalke commented at 8:11 am on May 31, 2021: 
    0        return QDateTime::fromSecsSinceEpoch(qint64{rec->time});
    fanquake commented at 5:17 am on June 7, 2021:
    Reverted per #349 (review).
  5. in src/qt/transactiontablemodel.cpp:633 in 01ba23e48f outdated 
    629@@ -630,7 +630,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
630     case TxPlainTextRole:
631         {
632             QString details;
633-            QDateTime date = QDateTime::fromTime_t(static_cast<uint>(rec->time));
634+            QDateTime date = QDateTime::fromSecsSinceEpoch(static_cast<uint>(rec->time));
    MarcoFalke commented at 8:11 am on May 31, 2021:
    same
    fanquake commented at 5:17 am on June 7, 2021:
    same
  6. MarcoFalke commented at 8:12 am on May 31, 2021: contributor
    You are passing the wrong type. Otherwise Concept ACK
  7. in src/qt/transactionfilterproxy.cpp:15 in 01ba23e48f outdated 
     9@@ -10,9 +10,9 @@
10 #include <cstdlib>
11 
12 // Earliest date that can be represented (far in the past)
13-const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromTime_t(0);
14+const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromSecsSinceEpoch(0);
15 // Last date that can be represented (far in the future)
16-const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromTime_t(0xFFFFFFFF);
17+const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromSecsSinceEpoch(0xFFFFFFFF);
    hebasto commented at 8:19 am on May 31, 2021:
    If @MarcoFalke’s suggestions will be addressed, the max qint64 could be also fixed.
    MarcoFalke commented at 6:06 am on June 3, 2021: 
    0const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromSecsSinceEpoch(std::numeric_limits<qint64>::max());

    not sure if this compiles

    promag commented at 8:59 am on June 3, 2021:
    I’m proposing to drop these in #354.
    hebasto commented at 9:20 am on June 6, 2021:

    not sure if this compiles

    It does.

    Also the <limits> header could be included:

     0--- a/src/qt/transactionfilterproxy.cpp
 1+++ b/src/qt/transactionfilterproxy.cpp
 2@@ -8,11 +8,12 @@
 3 #include <qt/transactionrecord.h>
 4 
 5 #include <cstdlib>
 6+#include <limits>
 7 
 8 // Earliest date that can be represented (far in the past)
 9 const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromSecsSinceEpoch(0);
10 // Last date that can be represented (far in the future)
11-const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromSecsSinceEpoch(0xFFFFFFFF);
12+const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromSecsSinceEpoch(std::numeric_limits<qint64>::max());
13 
14 TransactionFilterProxy::TransactionFilterProxy(QObject *parent) :
15     QSortFilterProxyModel(parent),
  8. fanquake force-pushed on Jun 1, 2021
  9. promag commented at 8:37 pm on June 2, 2021: contributor
    Concept ACK. No longer scripted-diff.
  10. fanquake renamed this:
    scripted-diff: replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch
    replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch
    on Jun 3, 2021
  11. MarcoFalke approved
  12. MarcoFalke commented at 6:06 am on June 3, 2021: contributor
    ACK
  13. in src/qt/transactiontablemodel.cpp:613 in ac577c573a outdated 
    609@@ -610,7 +610,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
610     case TypeRole:
611         return rec->type;
612     case DateRole:
613-        return QDateTime::fromTime_t(static_cast<uint>(rec->time));
614+        return QDateTime::fromSecsSinceEpoch(qint64{rec->time});
  14. in src/qt/transactiontablemodel.cpp:633 in ac577c573a outdated 
    629@@ -630,7 +630,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
630     case TxPlainTextRole:
631         {
632             QString details;
633-            QDateTime date = QDateTime::fromTime_t(static_cast<uint>(rec->time));
634+            QDateTime date = QDateTime::fromSecsSinceEpoch(qint64{rec->time});
  15. hebasto changes_requested
  16. fanquake force-pushed on Jun 7, 2021
  17. fanquake force-pushed on Jun 7, 2021
  18. MarcoFalke commented at 5:40 am on June 7, 2021: contributor
    cr ACK 4f5e9feb64400948223ae153b74eb41811f53cff
  19. fanquake force-pushed on Jun 7, 2021
  20. fanquake commented at 6:37 am on June 7, 2021: member
    Added one additional commit to replace QDateTime::toTime_t with QDateTime::toSecsSinceEpoch.
  21. hebasto approved
  22. hebasto commented at 12:24 pm on June 7, 2021: member

    ~ACK bd533e785903af0ba04cb0511c907b98008b056c,~ (will look into CI failures)

    0$ git grep fromTime_t | wc -l
10
2$ git grep toTime_t | wc -l
30

    Maybe update the OP to mention the second commit changes?

  23. in src/qt/recentrequeststablemodel.cpp:237 in bd533e7859 outdated 
    233@@ -234,7 +234,7 @@ bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, cons
234     switch(column)
235     {
236     case RecentRequestsTableModel::Date:
237-        return pLeft->date.toTime_t() < pRight->date.toTime_t();
238+        return pLeft->date.toSecsSinceEpoch() < pRight->date.toSecsSinceEpoch();
    Talkless commented at 6:38 pm on June 17, 2021:
    Out of scope, but I wonder why bool QDateTime::operator< is not used directly, without toTimeT()/toSecsSinceEpoch() ?
  24. in src/qt/transactionfilterproxy.cpp:16 in bd533e7859 outdated 
    13 // Earliest date that can be represented (far in the past)
14-const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromTime_t(0);
15+const QDateTime TransactionFilterProxy::MIN_DATE = QDateTime::fromSecsSinceEpoch(0);
16 // Last date that can be represented (far in the future)
17-const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromTime_t(0xFFFFFFFF);
18+const QDateTime TransactionFilterProxy::MAX_DATE = QDateTime::fromSecsSinceEpoch(std::numeric_limits<qint64>::max());
    Talkless commented at 6:38 pm on June 17, 2021:

    From Qt documentation [0]:

    Note that there are possible values for secs that lie outside the valid range of QDateTime, both negative and positive. The behavior of this function is undefined for those values.

    Anyone knows what are these “ouside” values, and is this ::max() OK here then?

    [0] https://doc.qt.io/qt-5/qdatetime.html#fromSecsSinceEpoch

    Talkless commented at 1:10 pm on August 11, 2021:
    I guess we have to wait for #354 to be merged, these lines will go away.
  25. Talkless commented at 6:39 pm on June 17, 2021: none
    Concept ACK
  26. hebasto added the label Refactoring on Jun 28, 2021
  27. DrahtBot added the label Needs rebase on Aug 11, 2021
  28. fanquake force-pushed on Aug 15, 2021
  29. DrahtBot removed the label Needs rebase on Aug 15, 2021
  30. MarcoFalke commented at 10:49 am on August 23, 2021: contributor
    cr ACK bfcde3e87231d90bdacd3a8154e2be16a52508c7
  31. in src/qt/recentrequeststablemodel.h:30 in bfcde3e872 outdated 
    25@@ -26,9 +26,9 @@ class RecentRequestEntry
26 
27     SERIALIZE_METHODS(RecentRequestEntry, obj) {
28         unsigned int date_timet;
    MarcoFalke commented at 10:50 am on August 23, 2021:
    unrelated: This is still the wrong type
    fanquake commented at 1:45 am on August 24, 2021:
    Changed the type here.
  32. in src/qt/transactionfilterproxy.cpp:11 in bfcde3e872 outdated 
     7@@ -8,6 +8,7 @@
 8 #include <qt/transactionrecord.h>
 9 
10 #include <cstdlib>
11+#include <limits>
    hebasto commented at 9:00 pm on August 23, 2021:
    This change is unrelated after the recent rebase and could be dropped.
    fanquake commented at 1:26 am on August 24, 2021:
    Replaced it with the headers that are actually used.
  33. hebasto commented at 9:01 pm on August 23, 2021: member
    Approach ACK bfcde3e87231d90bdacd3a8154e2be16a52508c7.
  34. fanquake force-pushed on Aug 24, 2021
  35. fanquake force-pushed on Aug 24, 2021
  36. hebasto commented at 6:52 am on August 24, 2021: member 
    0./serialize.h:676:6: error: member reference base type 'long long' is not a structure or union
1    a.Unserialize(is);
2    ~^~~~~~~~~~~~
  37. refactor: replace QDateTime::fromTime_t with QDateTime::fromSecsSinceEpoch 27257b39bf
  38. refactor: replace QDateTime::toTime_t with QDateTime::toSecsSinceEpoch 3ae503c95b
  39. in src/qt/recentrequeststablemodel.h:30 in b237992fc4 outdated 
    26@@ -25,10 +27,10 @@ class RecentRequestEntry
27     SendCoinsRecipient recipient;
28 
29     SERIALIZE_METHODS(RecentRequestEntry, obj) {
30-        unsigned int date_timet;
31-        SER_WRITE(obj, date_timet = obj.date.toTime_t());
32+        qint64 date_timet;
    MarcoFalke commented at 7:02 am on August 24, 2021:
    I don’t think this can be changed without breaking wallet.dat
    fanquake commented at 7:03 am on August 24, 2021:
    I’m going to revert this. We can leave it as unrelated.
  40. fanquake force-pushed on Aug 24, 2021
  41. hebasto approved
  42. hebasto commented at 7:14 am on August 24, 2021: member
    ACK 3ae503c95bccc4899fc881c603d618a824078e7b
  43. MarcoFalke merged this on Aug 24, 2021
  44. MarcoFalke closed this on Aug 24, 2021
  45. fanquake deleted the branch on Aug 24, 2021
  46. sidhujag referenced this in commit f993eafecd on Aug 24, 2021
  47. bitcoin-core locked this on Aug 24, 2022


fanquake hebasto MarcoFalke promag Talkless

Labels
Refactoring

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-23 07:20 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me