Skip to content

Conversation

@ecrupper
Copy link
Contributor

It looks like these two unchecked unmarshal calls were part of the initial go-vela/types commit, so I am not sure if there is some extra backstory here that I'm missing. However, it would be nice to bubble up errors here, since suppressing them just returns an empty rules block which gets converted to no rules.

ruleset:
  branch:
  event: push
  branch: main

converts to -->

Before:

ruleset:
  operator: and
  matcher: filepath

After:

error: "mapping key \"branch\" already defined"

FWIW, the unmarshaler is not strict on fields, so this won't break pipelines that are using unknown fields (threw in a test just to confirm).

@ecrupper ecrupper requested a review from a team as a code owner February 18, 2025 21:40
@codecov
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.60%. Comparing base (1e93bf1) to head (9260f1e).
Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (56.60%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1256   +/-   ##
=======================================
  Coverage   56.60%   56.60%           
=======================================
  Files         629      629           
  Lines       35653    35654    +1     
=======================================
+ Hits        20180    20181    +1     
  Misses      14792    14792           
  Partials      681      681           
Files with missing lines Coverage Δ
compiler/types/yaml/yaml/ruleset.go 63.18% <100.00%> (+0.18%) ⬆️

@ecrupper ecrupper merged commit 1b4406b into main Feb 21, 2025
11 of 13 checks passed
@ecrupper ecrupper deleted the bubble-rules-unmarshal-errors branch February 21, 2025 16:37
wass3rw3rk added a commit that referenced this pull request Mar 10, 2025
wass3rw3rk added a commit that referenced this pull request Mar 10, 2025
* Revert "fix(compiler): validate step naming conflicts (#1257)"

This reverts commit 1f6a9b9.

* Revert "fix(yaml): bubble up rules struct unmarshal errors (#1256)"

This reverts commit 1b4406b.
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.

4 participants