Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not list changed dependencies in summary #828

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Do not list changed dependencies in summary #828

merged 4 commits into from
Sep 16, 2024

Conversation

hmaurer
Copy link
Contributor

@hmaurer hmaurer commented Sep 13, 2024

Closes #786.

Problem

Right now we list every dependency that was either added or removed in the Markdown job summary. For pull requests with large number of changes, this goes over the summary size limit, as reported in #786.

Changes in this pull request

I removed the list of changed dependencies from the job summary. They can still be found in the logs. My rationale for this change is that the list of changed dependencies is likely not what users most care about in the summary: the list of vulnerable dependencies, license infringements and such are what's key. The way we were currently listing the dependency changes in the summary was not of much value either way as we were not differentiating between added dependencies and removed dependencies, we were listing all of them (whether added or removed).

This is not the most elegant way to address this issue, but it is the most straightforward.

@hmaurer hmaurer requested a review from a team as a code owner September 13, 2024 13:14
@hmaurer hmaurer changed the title Do not list changes dependencies in summary Do not list changed dependencies in summary Sep 13, 2024
@hmaurer hmaurer force-pushed the hm/summary branch 4 times, most recently from 218bc59 to 056b129 Compare September 13, 2024 13:30
Copy link
Collaborator

@jonjanego jonjanego left a comment

Choose a reason for hiding this comment

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

do you have a test run of this i could see?

src/comment-pr.ts Outdated Show resolved Hide resolved
scripts/create_summary.ts Show resolved Hide resolved
@elireisman elireisman self-assigned this Sep 16, 2024
@elireisman elireisman merged commit 6ea3b24 into main Sep 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Job Summary Size Limitation aborts the job [BUG]
3 participants