Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29887585: Issue #5352 - generate_static_pages cannot deal with directories being turned into regular pages (Closed)

Created:
Sept. 21, 2018, 1:35 p.m. by Tudor Avram
Modified:
Oct. 17, 2018, 2:47 p.m.
Reviewers:
Vasily Kuznetsov
Visibility:
Public.

Description

Issue #5352 - generate_static_pages cannot deal with directories being turned into regular pages

Patch Set 1 #

Total comments: 20

Patch Set 2 : Addressed comments from Patch #1 #

Total comments: 10

Patch Set 3 : Addressed comments from Patch Set #2 #

Patch Set 4 : Fixed issue causing tests to fail in Patch Set #3 #

Patch Set 5 : Addressed comments from Patch Set #4 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -10 lines) Patch
M .gitignore View 1 chunk +2 lines, -0 lines 0 comments Download
M .hgignore View 1 1 chunk +1 line, -0 lines 0 comments Download
M cms/bin/generate_static_pages.py View 1 2 3 3 chunks +89 lines, -10 lines 0 comments Download
M tests/expected_output/en/sitemap View 1 chunk +1 line, -0 lines 0 comments Download
A tests/test_generation_exceptional_cases.py View 1 2 3 4 1 chunk +71 lines, -0 lines 0 comments Download
A tests/test_site/pages/foo/bar.md View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M tox.ini View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12
Tudor Avram
Hi Vasily, Here is my fix for the issue for raising an exception when turning ...
Sept. 21, 2018, 1:37 p.m. (2018-09-21 13:37:34 UTC) #1
Vasily Kuznetsov
Hi Tudor! The overall approach looks good. I have some comments on code details though ...
Sept. 24, 2018, 11:58 a.m. (2018-09-24 11:58:47 UTC) #2
Tudor Avram
Hi Vasily, Thanks for your feedback! I just had a small comment on the `makedirs()` ...
Sept. 24, 2018, 1:26 p.m. (2018-09-24 13:26:45 UTC) #3
Tudor Avram
Also, one more question (see below): https://codereview.adblockplus.org/29887585/diff/29887586/.gitignore File .gitignore (right): https://codereview.adblockplus.org/29887585/diff/29887586/.gitignore#newcode9 .gitignore:9: .idea/ On 2018/09/24 ...
Sept. 24, 2018, 2:04 p.m. (2018-09-24 14:04:52 UTC) #4
Vasily Kuznetsov
Hi Tudor, > Also, from looking at the os.makedirs() code, it looks like it would ...
Sept. 24, 2018, 2:42 p.m. (2018-09-24 14:42:23 UTC) #5
Tudor Avram
Hi Vasily, I addressed your comments from the previous patch. I also refactored a couple ...
Sept. 24, 2018, 4:10 p.m. (2018-09-24 16:10:00 UTC) #6
Vasily Kuznetsov
Hi Tudor! Almost there. Just a few minor comments. Cheers, Vasily https://codereview.adblockplus.org/29887585/diff/29890605/cms/bin/generate_static_pages.py File cms/bin/generate_static_pages.py (right): ...
Sept. 27, 2018, 11:05 a.m. (2018-09-27 11:05:08 UTC) #7
Tudor Avram
Hi Vasily, I addressed all of your comments (except removing `script_runner` for one of the ...
Oct. 4, 2018, 6:34 a.m. (2018-10-04 06:34:19 UTC) #8
Tudor Avram
Hi Vasily, Really sorry, just dicovered that my previous change broke a couple of tests. ...
Oct. 4, 2018, 9:56 a.m. (2018-10-04 09:56:41 UTC) #9
Vasily Kuznetsov
Hi Tudor! Let's also convert test_generate_fifo_instead_of_file and then it's done (see details below). Cheers, Vasily ...
Oct. 5, 2018, 9:40 a.m. (2018-10-05 09:40:44 UTC) #10
Tudor Avram
Hi Vasily, Here is the change for the last test. Sorry again for all this ...
Oct. 5, 2018, 11:28 a.m. (2018-10-05 11:28:34 UTC) #11
Vasily Kuznetsov
Oct. 5, 2018, 11:42 a.m. (2018-10-05 11:42:40 UTC) #12
LGTM

Powered by Google App Engine
This is Rietveld