feat: introduce template renderer [CLI-506] #252
Conversation
thisislawatts
commented
Oct 1, 2024
•
thisislawatts
commented
- feat: introduce localfinding presenter CLI-506
- test: add failing check for template contents
| log.Warn().Err(err).Msg("Failed to unmarshal local finding") | ||
| continue | ||
| } | ||
| findingPresentation := presenters.LocalFindingPresentation{ |
There was a problem hiding this comment.
Suggestion: The fact that we have to add the path to an extra structure means for me, that this information is missing in the local findings model. The path is not a presenter specific concern but a test concern hence it should be in the findings model.
| @@ -1,3 +1,3 @@ | |||
| package local_models | |||
|
|
|||
| //go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen -config oapi.config.yaml ../cueutils/source/openapi/rest/test.spec.yaml | |||
| //go:generate oapi-codegen -config oapi.config.yaml ../../../internal/cueutils/source/openapi/rest/test.spec.yaml | |||
There was a problem hiding this comment.
Question: this and other changes are already on main, why are they here?
There was a problem hiding this comment.
Without a rebase, it's hard to tell if I'm reviewing things that have already landed in main, or the topic of this PR.
The summary template coverage looks good. I love the box custom template function.
I'd like to see more test coverage of rendering individual findings.
internal/api/api.go
Outdated
| @@ -163,6 +163,7 @@ func (a *snykApiClient) GetFeatureFlag(flagname string, orgId string) (bool, err | |||
| } | |||
|
|
|||
| var flag contract.OrgFeatureFlagResponse | |||
| flag.Ok = defaultResult | |||
There was a problem hiding this comment.
false is already the default bool value in Go, is this needed?
internal/api/api_test.go
Outdated
There was a problem hiding this comment.
Suggest adding a GetFeatureFlag error case as well, to make sure it returns the default (not ok).
There was a problem hiding this comment.
(This might be a drive-by request, if this is something already in latest main pending a rebase)
j-luong
force-pushed
the
feat/CLI-506-introduce-template-renderer
branch
from
163e38f
to
120e54a
Compare
