Skip to content

feat(ci): check that Kontrol summary files are unchanged#12592

Merged
smartcontracts merged 1 commit intodevelopfrom
sc/ci-check-kontrol-summaries
Oct 26, 2024
Merged

feat(ci): check that Kontrol summary files are unchanged#12592
smartcontracts merged 1 commit intodevelopfrom
sc/ci-check-kontrol-summaries

Conversation

@smartcontracts
Copy link
Copy Markdown
Contributor

Updates CI to check that the Kontrol summary dummy files are not modified by any PR unless the change is deliberate. Committed files are dummy files and aren't meant to change.

Alternative here would be to generate the dummy files with a script that gets triggered before forge builds stuff. Main concern with using an autogen script is that the script would need to be added in front of build commands all over the place. Committing the files and checking that they aren't changed is slightly more janky but people rarely re-generate these files anyway so the projected impact is minimal.

If this becomes a devx hassle we can consider autogen.

Updates CI to check that the Kontrol summary dummy files are not
modified by any PR unless the change is deliberate. Committed files
are dummy files and aren't meant to change.

Alternative here would be to generate the dummy files with a
script that gets triggered before forge builds stuff. Main concern
with using an autogen script is that the script would need to be
added in front of build commands all over the place. Committing
the files and checking that they aren't changed is slightly more
janky but people rarely re-generate these files anyway so the
projected impact is minimal.

If this becomes a devx hassle we can consider autogen.
Copy link
Copy Markdown
Contributor

@AmadiMichael AmadiMichael left a comment

Choose a reason for hiding this comment

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

LGTM!

@smartcontracts smartcontracts added this pull request to the merge queue Oct 26, 2024
Merged via the queue into develop with commit 44f814c Oct 26, 2024
@smartcontracts smartcontracts deleted the sc/ci-check-kontrol-summaries branch October 26, 2024 05:50
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.86%. Comparing base (8311719) to head (2db0c76).
Report is 41 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12592      +/-   ##
===========================================
- Coverage    65.04%   64.86%   -0.18%     
===========================================
  Files           54       54              
  Lines         4460     4460              
===========================================
- Hits          2901     2893       -8     
- Misses        1382     1391       +9     
+ Partials       177      176       -1     
Flag Coverage Δ
cannon-go-tests 64.86% <ø> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

samlaf pushed a commit to samlaf/optimism that referenced this pull request Nov 10, 2024
…timism#12592)

Updates CI to check that the Kontrol summary dummy files are not
modified by any PR unless the change is deliberate. Committed files
are dummy files and aren't meant to change.

Alternative here would be to generate the dummy files with a
script that gets triggered before forge builds stuff. Main concern
with using an autogen script is that the script would need to be
added in front of build commands all over the place. Committing
the files and checking that they aren't changed is slightly more
janky but people rarely re-generate these files anyway so the
projected impact is minimal.

If this becomes a devx hassle we can consider autogen.
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.

3 participants