Differential D20929

fix compilation to not require filesystem stl c++ header
ClosedPublic
Actions

Authored by mgallien on Apr 30 2019, 10:04 PM.

Details

Reviewers
• jriddell
ngraham
pino
Group Reviewers
Elisa
Commits
R255:4bedccca8f21: fix compilation to not require filesystem stl c++ header
Summary

Use QDir::rename on platforms where filesystem stl standard header might not be present

Test Plan

Automatic tests are OK

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgallien requested review of this revision.Apr 30 2019, 10:04 PM
mgallien created this revision.
ngraham accepted this revision.May 1 2019, 2:20 AM
ngraham added a subscriber: ngraham.

Seems sensible.

This revision is now accepted and ready to land.May 1 2019, 2:20 AM
pino added a subscriber: pino.May 1 2019, 7:27 AM

Or, even better, just unconditionally use QDir::rename? This way there is no need to maintain two different code paths with a number of arbitrary #ifdef blocks.

mgallien added a comment.May 1 2019, 11:43 AM
In D20929#458881, @pino wrote:

Or, even better, just unconditionally use QDir::rename? This way there is no need to maintain two different code paths with a number of arbitrary #ifdef blocks.

I even thought about doing that. I am pretty sure you are right.

mgallien updated this revision to Diff 57308.May 1 2019, 12:13 PM
  • remove the #if

always use QDir::rename

pino added inline comments.May 1 2019, 12:45 PM
autotests/localfilelistingtest.cpp
388–389

Even easier: QDir().rename(...);

mgallien updated this revision to Diff 57316.May 1 2019, 1:48 PM
  • simplify the QDir::rename usage
pino accepted this revision.May 1 2019, 2:03 PM

Looks ok now, thanks for the changes.

mgallien added a comment.May 1 2019, 2:03 PM
In D20929#458998, @pino wrote:

Looks ok now, thanks for the changes.

It is better now. Thanks for the review.

Closed by commit R255:4bedccca8f21: fix compilation to not require filesystem stl c++ header (authored by mgallien). · Explain WhyMay 1 2019, 2:04 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mautotests/localfilelistingtest.cpp (11 lines)

Diff 57317

View Options

autotests/localfilelistingtest.cpp

Loading...