Dedupe GetFilesWithSuffix and document hash struct precondition - #2690
Open
abrarshivani wants to merge 2 commits into
Open
Dedupe GetFilesWithSuffix and document hash struct precondition#2690abrarshivani wants to merge 2 commits into
abrarshivani wants to merge 2 commits into
Conversation
GetFilesWithSuffix appended a path once per matching suffix, so a file that matched more than one of the provided suffixes was returned multiple times. Break after the first matching suffix so each file is returned at most once; this is behavior-identical for non-overlapping suffixes (the only current caller passes yaml/yml/json). Add a test asserting the single result. Also document that GetObjectHashIgnoreEmptyKeys requires a struct (or pointer to struct) and panics on other kinds, matching what all callers already pass. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
5 tasks
Cover the break-on-first-match path fully: order-independence, single-of-many match, multiple overlapping-match files, non-matching exclusion, and recursion. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
abrarshivani
marked this pull request as ready for review
July 28, 2026 23:38
abrarshivani
requested review from
cdesiniotis,
karthikvetrivel,
rahulait,
rajathagasthya,
shivamerla and
tariq1890
as code owners
July 28, 2026 23:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two small
internal/utilsproduction changes split out of the test-coverage PR #2656 (per review, so production behavior is tracked on its own):GetFilesWithSuffixappended a path once per matching suffix, so a file matching more than one of the provided suffixes was returned multiple times. It nowbreaks after the first matching suffix, returning each file at most once. Behavior-identical for the only current caller (which passes the non-overlappingyaml/yml/json); the change guards against future overlapping-suffix callers.GetObjectHashIgnoreEmptyKeys— documented (no behavior change) that the argument must be a struct or pointer-to-struct, and that other kinds/nil panic, since only struct fields can be enumerated. Matches what all callers already pass.A test asserts
GetFilesWithSuffixreturns a single entry for a file matching multiple suffixes.Checklist
make lint)make validate-generated-assets)make validate-modules)Testing