Skip to content

avoid that '--inject-checksums' adds lines longer than 120 characters#2434

Merged
boegel merged 5 commits intoeasybuilders:developfrom
smoors:checksum_comment
Mar 13, 2018
Merged

avoid that '--inject-checksums' adds lines longer than 120 characters#2434
boegel merged 5 commits intoeasybuilders:developfrom
smoors:checksum_comment

Conversation

@smoors
Copy link
Copy Markdown
Contributor

@smoors smoors commented Mar 10, 2018

if the line is too long, put the comment in a separate line before the checksum.

@boegel boegel changed the title fix checksum lines > 120 chars avoid that --inject-checksums adds lines longer than 120 characters Mar 11, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 11, 2018

@smoors We should do the same for checksums for extensions, and also enhance the existing test for --inject-checksums to cover the new code. I looked into this, see smoors#1, please review/merge to update this PR.

@boegel boegel added this to the 3.6.0 milestone Mar 11, 2018
extend enhanced `--inject-checksums` to extensions + enhance test
Comment thread easybuild/framework/easyblock.py Outdated
"%s'%s'," % (line_indent, checksum),
])
else:
exts_list_lines.append(ext_checksum_line)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoors This diff makes it really stand out that this is a whole bunch of duplicate code, which we should avoid...

We should look into adding an (inner?) function that takes care of this once, for a specified indent level, so we can use it in both situations...

Let me know if you're up for looking into that or not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I will look into it.

Comment thread test/framework/easyblock.py Outdated
# reset
eb.patches = []

toy_patch = 'toy-0.0_fix-silly-typo-in-printf-statement.patch'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should be moved up I guess so toy_patch can also be used above in this test, I overlooked that.

@boegel boegel changed the title avoid that --inject-checksums adds lines longer than 120 characters avoid that '--inject-checksums' adds lines longer than 120 characters Mar 13, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 13, 2018

Thanks a lot for looking into this @smoors!

@boegel boegel merged commit ee386f4 into easybuilders:develop Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants