Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(101)

Issue 29538640: Issue 5610 - Deal with a '/' base path (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by hub
Modified:
2 years, 5 months ago
Reviewers:
sergei
Base URL:
https://hg.adblockplus.org/libadblockplus/
Visibility:
Public.

Description

Issue 5610 - Deal with a '/' base path

Patch Set 1 #

Total comments: 7

Patch Set 2 : Changes from review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -2 lines) Patch
M src/DefaultFileSystem.cpp View 2 chunks +5 lines, -2 lines 0 comments Download
M test/DefaultFileSystem.cpp View 1 2 chunks +52 lines, -0 lines 0 comments Download

Messages

Total messages: 5
hub
2 years, 5 months ago (2017-09-07 16:31:42 UTC) #1
hub
This include tests.
2 years, 5 months ago (2017-09-07 17:10:33 UTC) #2
sergei
Good, merely a couple of things in tests. https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSystem.cpp File test/DefaultFileSystem.cpp (right): https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSystem.cpp#newcode39 test/DefaultFileSystem.cpp:39: class ...
2 years, 5 months ago (2017-09-08 08:11:40 UTC) #3
hub
updated patch https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSystem.cpp File test/DefaultFileSystem.cpp (right): https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSystem.cpp#newcode39 test/DefaultFileSystem.cpp:39: class BasePathTest : public ::testing::Test On 2017/09/08 ...
2 years, 5 months ago (2017-09-08 13:10:26 UTC) #4
sergei
2 years, 5 months ago (2017-09-08 13:20:19 UTC) #5
LGTM

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
File test/DefaultFileSystem.cpp (right):

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:84: #define SLASH_STRING "/"
On 2017/09/08 13:10:26, hub wrote:
> On 2017/09/08 08:11:40, sergei wrote:
> > These defines are already available as PATH_SEPARATOR from
> > "../src/DefaultFileSystem.h" which is included in this file.
> 
> but they are single char, while this is a string.

Acknowledged.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5