Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (8)
@ximion ximion Apr 29, 2020
style: Please align parameters after a line break with the other parameters: ``` override Package[] packagesFor(string suite, string section, string arch, bool withLongDescs = true) { ``` In this case you probably don't even need to break the line though (with D's ultra-verbose function decorators, having space here is a good thing)
Outdated
src/asgen/backends/alpinelinux/apkpkgindex.d
@ximion ximion Apr 29, 2020
style: Please don't add a linebreak here - long lines are perfectly acceptable, if they are readable (no 80char limit applies).
Outdated
src/asgen/backends/alpinelinux/apkpkgindex.d
@ximion ximion Apr 29, 2020
You potentially want to filter this to exclude the `.PKGINFO` file and the signature file (not absolutely required, but may be the nice thing to do - otherwise asgen will assume these files are actually installed with every package).
Outdated
src/asgen/backends/alpinelinux/apkpkgindex.d
Cogitri
@ximion ximion Apr 29, 2020
This is ugly - if `i` went over the array limit and is used anywhere after this point, this will crash. One possible fix is not not look ahead at the next line, but instead remember the current value until reading the next line and to only finalize the value when that line doesn't contain an equal sign. That way, you can save a loop and get rid of all index variables by using a foreach loop for `lines`.
Outdated
src/asgen/backends/alpinelinux/apkpkgindex.d
Cogitri
@ximion ximion Apr 29, 2020
style: Don't use C-style comment blocks unless needed, and don't mix comment styles (there's multiple styles of comments in this file)
Outdated
src/asgen/backends/alpinelinux/apkpkgindex.d
@ximion ximion Apr 29, 2020
style: Skip braces for simple one-line statements after a conditional (especially if it's a return statement)
src/asgen/backends/alpinelinux/apkpkgindex.d
@ximion ximion Apr 29, 2020
style: For anything but functions, curly braces should be in-line with the keyword (this is in multiple occasions, so I'll not mark every spot)
Outdated
src/asgen/backends/alpinelinux/apkpkg.d
@ximion ximion Apr 29, 2020
style: Space between function name and parameter brackets (like `void funcName ()`) This also applies to invocations: `x = foo (bar)` - It does *not* apply to template instantiations though: `y = baz!("abc", "def")` (this is in multiple occasions, so I'll not mark every spot)
Outdated
src/asgen/backends/alpinelinux/apkpkg.d