Please provide a branch that is based on a current dev with only changes for this ticket on it. E.g. not the old 42cc tickets from weeks ago, nor ticket:21
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks for repushing the branch. Sorry this has taken a while to get to reviewing. Here's a few things to tweak:
the <h3> includes the whole file content, which is not correct
let's have a unit test for generate_code_stats()
what happens with unicode text? Won't len() return a character count and be incorrect for a file size? It'd be nicer to get the file size more directly, so that encodings don't affect it.
I think the data_line_count would be a bit more accurate if you skip lines with just whitespace. E.g. in many cases there are lines with just spaces. A precompiled regex using \s would work well I think.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Closed #41 and pushed changes into 42cc_4126
Please provide a branch that is based on a current dev with only changes for this ticket on it. E.g. not the old 42cc tickets from weeks ago, nor ticket:21
Must be some mistake... I'll look into it.
Cherry picked a commit related to the ticket, removed the branch and pushed again. Please check.
The ticket #41 was previously #21 before split, that's why probably the branch was based on ticket:21
Thanks for repushing the branch. Sorry this has taken a while to get to reviewing. Here's a few things to tweak:
<h3>
includes the whole file content, which is not correct\s
would work well I think.OK, I've created #71: [#4126] Tests, fixes and amendments (1cp) to fix this
Related
Tickets:
#4126closed #71, changes in 42cc_4126