Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date

[David Faure]

> On Jan. 7, 2016, 11:15 a.m., David Faure wrote:
> > src/ioslaves/ftp/ftp.cpp, line 1775
> > 
> >
> > The porting bug is here. In kdelibs4 tmptr was initialized to the 
> > current date, and used below.
> > 
> > Now we have two variables, "int year" and "currentTime.year".
> > 
> > To be closer to the orig code and to avoid risking that the day or 
> > month is still 0 as well, I would suggest to initialize year, day and month 
> > directly from currentTime, rather than to 0.
> > 
> > I guess we can also remove the "currentTime.setTime(QTime(0,0,0)) 
> > because that's unused (right?)
> 
> Wolfgang Bauer wrote:
> > To be closer to the orig code and to avoid risking that the day or 
> month is still 0 as well, I would suggest to initialize year, day and month 
> directly from currentTime, rather than to 0.
> 
> Well, if we don't know the values, taking them from currentTime is likely 
> just as wrong as setting them to 0.
> But ok, I'll change it.
> 
> > I guess we can also remove the "currentTime.setTime(QTime(0,0,0)) 
> because that's unused (right?)
> 
> Right, it is unused AFAICS.
> Actually we could also just use QDate::currentDate() instead of 
> QDateTime::currentDateTime() IMHO.

"likely just as wrong" - well with 0 the QDateTime would be invalid, making the 
whole information disappear (even e.g. the valid time information). So using 
the current datetime (as kdelibs4 did) seems better.

Agreed about currentDate().


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126659/#review90743
---


On Jan. 7, 2016, 12:23 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126659/
> ---
> 
> (Updated Jan. 7, 2016, 12:23 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and David Faure.
> 
> 
> Bugs: 354597
> https://bugs.kde.org/show_bug.cgi?id=354597
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so 
> subtracting 1900 is wrong.
> - Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only 
> need the current date anyway
> - Initialize day, month, and year to the current date instead of 0. In the 
> case when no year is mentioned in the server's reply (the year is implicit), 
> it wasn't set to the current year at all, so the result was either 0 or -1.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/ftp/ftp.cpp 2179179 
> 
> Diff: https://git.reviewboard.kde.org/r/126659/diff/
> 
> 
> Testing
> ---
> 
> Connected to an FTP server with dolphin (15.12.0). The modification 
> times/dates are now shown correctly.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126659/#review90750
---

Ship it!


Ship It!

- David Faure


On Jan. 7, 2016, 12:23 p.m., Wolfgang Bauer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126659/
> ---
> 
> (Updated Jan. 7, 2016, 12:23 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs and David Faure.
> 
> 
> Bugs: 354597
> https://bugs.kde.org/show_bug.cgi?id=354597
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> - QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so 
> subtracting 1900 is wrong.
> - Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only 
> need the current date anyway
> - Initialize day, month, and year to the current date instead of 0. In the 
> case when no year is mentioned in the server's reply (the year is implicit), 
> it wasn't set to the current year at all, so the result was either 0 or -1.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/ftp/ftp.cpp 2179179 
> 
> Diff: https://git.reviewboard.kde.org/r/126659/diff/
> 
> 
> Testing
> ---
> 
> Connected to an FTP server with dolphin (15.12.0). The modification 
> times/dates are now shown correctly.
> 
> 
> Thanks,
> 
> Wolfgang Bauer
> 
>

Re: Review Request 126659: [kio_ftp] fix display of file/directory modification time/date

[Wolfgang Bauer]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126659/
---

(Updated Jan. 7, 2016, 12:46 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kdelibs and David Faure.


Changes
---

Submitted with commit 68af1d7e89b7fed136d4cc62b76c1c6ded2d94eb by Wolfgang 
Bauer to branch master.


Bugs: 354597
https://bugs.kde.org/show_bug.cgi?id=354597


Repository: kio


Description
---

- QDate() treats the year literally (i.e. 90 is really year 90, not 1990), so 
subtracting 1900 is wrong.
- Use QDate::currentDate() instead of QDateTime::currentDateTime(), we only 
need the current date anyway
- Initialize day, month, and year to the current date instead of 0. In the case 
when no year is mentioned in the server's reply (the year is implicit), it 
wasn't set to the current year at all, so the result was either 0 or -1.


Diffs
-

  src/ioslaves/ftp/ftp.cpp 2179179 

Diff: https://git.reviewboard.kde.org/r/126659/diff/


Testing
---

Connected to an FTP server with dolphin (15.12.0). The modification times/dates 
are now shown correctly.


Thanks,

Wolfgang Bauer