avoid that --inject-checksums introduces list of patches for extensions as a single long line (fixes #2734)#3025
Conversation
9def0be to
4fdc285
Compare
…ns as a single long line
4fdc285 to
05300af
Compare
| val = quote_str(val, prefer_single_quotes=True) | ||
| exts_list_lines.append("%s'%s': %s," % (INDENT_4SPACES * 2, key, val)) | ||
| strval = quote_str(val, prefer_single_quotes=True) | ||
| line = "%s'%s': %s," % (INDENT_4SPACES * 2, key, strval) |
There was a problem hiding this comment.
When are we dropping python 2 support? This would look so much better with f-strings :-P
There was a problem hiding this comment.
It's not that bad... It's not pretty, sure, but does it have to be?
We use %s string templating pretty consistently across the codebase, which is more important imho...
When we drop Python 2 support (perhaps with EasyBuild 5, time will tell), I'll count on you to port all of that to f-strings so we can keep things consistent. ;)
Would this be better?
line = INDENT_4SPACES * 2 + "'" + key + "': " + strval + ","There was a problem hiding this comment.
That's much more readable IMO, but I am ok keeping the other approach. My comment was a general comment, nothing that have to be necessarily changed now.
There was a problem hiding this comment.
Nah, the "%s'%s': %s," is much easier to read. I'm a C-programmer by heart....
…h --inject-checksums
|
@akesandgren you were also requested for review. I can merge it right now, but since @boegel is counting on you I rather wait ;-) |
|
To be clear, I requested @akesandgren's review since the reported the bug being fixed here, not because we'd block the PR... ;) |
|
Going in, thanks @boegel! |
problem reported in #2734 reproduced in tests, and then fixed