feat: execute all steps and collect errors at end of chain#1640
feat: execute all steps and collect errors at end of chain#1640jescalada wants to merge 8 commits into
Conversation
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1640 +/- ##
==========================================
+ Coverage 85.75% 86.01% +0.25%
==========================================
Files 85 85
Lines 8448 8488 +40
Branches 1457 1479 +22
==========================================
+ Hits 7245 7301 +56
+ Misses 1172 1157 -15
+ Partials 31 30 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
coopernetes
left a comment
coopernetes
left a comment
There was a problem hiding this comment.
Overall LGTM, just a few notes/suggestions.
| * such as repository authorisation, user push permission, or later steps | ||
| * depending on their output (pullRemote, writePack, getDiff) | ||
| */ | ||
| const collectibleSteps = new Set<Processor['exec']>([ |
There was a problem hiding this comment.
Worth adding a class/object property like private readonly boolean isCollectible directly on the Processor type? Then each step can self-declare it's behaviour and this list doesn't need to remain statically defined (and needing updates when new checks are introduced).
There was a problem hiding this comment.
Could also be worth thinking about how plugin behave - they could be collectible or not...
kriswest
left a comment
kriswest
left a comment
There was a problem hiding this comment.
LGTM, although I agree with @coopernetes that the config option can be removed and that processors could self declare their behaviour, which could have an impact on plugins. Up to you if you deal with the alter point in this PR or raise a follow-up issue. However, best to drop the config setting now if we don't need it
|
@kriswest @coopernetes Ready for another look! I'm not sure about the plugin behavior - should these be collectible as well? I'm happy to open another PR for this to get this merged faster 👍🏼 |
3 participants
Description
This PR implements one of fogwall's quality-of-life features: collecting all errors in the chain and reporting to the user, so they don't have to make multiple pushes and fix one-by-one. This still fails mid-chain if non-validator steps (
parsePush,pullRemote,writePack, etc.) error out.collectibleStepsare defined inchain.ts(steps that can be collected and reported later, as opposed to immediately failing).Related Issue
Resolves #
Checklist
General
Documentation
Configuration
config.schema.json) was modified:npm run generate-config-types)npm run gen-schema-doc)Tests
npm test)npm run lintandnpm run format:check)npm run check-types)