Give patch file unique name and provide text of short diffs in job summary #7674
Conversation
mjschmidt271
added
BFB
bartgol
left a comment
bartgol
left a comment
There was a problem hiding this comment.
Looks good. I got some comments though.
| "branch." >> "${GITHUB_STEP_SUMMARY}" | ||
| echo "Then commit the changes and push to your remote branch." >> "${GITHUB_STEP_SUMMARY}" | ||
| echo "" >> "${GITHUB_STEP_SUMMARY}" | ||
| echo "For diffs with ***fewer than 10 files***, we also include the text of " \ |
There was a problem hiding this comment.
Some files may still have A LOT of diffs, especially the first time the files undergo clang-format... Maybe you could just run head -N 50 <diff_file> (or 100, or whatever many lines you're comfortable with)?
There was a problem hiding this comment.
Hmm... that makes sense. The only different approach I might take is to use the head -N <num> to determine whether we include the diff text. The reason being that a partial diff of a file is no use at all as a patch.
@mahf708 ?
There was a problem hiding this comment.
Do you think it's too much to ask that the user downloads the patch and looks at it? I think having a list of files that need fixing is plenty enough for the summary...
There was a problem hiding this comment.
That's ok, as I said, no strong feelings either way. I don't mind
mjschmidt271
force-pushed
the
mjs/eamxx/format-patch-in-summary
branch
from
9f76bb7 to
416f2d7
Compare
Give patch file unique name and provide text of short diffs in job summary [BFB]
Pretty simple, in theory. In practice... spent the last hour wrestling gh action syntax 😤😂