tool: cfg.GroupSubdirBy - group by dir or fname - #926
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Review: add GroupSubdirBy config option
The change is small, focused, and clean. Replacing the goFileOf bool parameter with a named iota enum (groupSubdirByNone/Dir/Fname) reads clearly, the empty-string + GroupSubdir=true backward-compat path is handled explicitly, and the default warning on bad input is a sensible defensive choice. No performance or clearly-exploitable security concerns were found.
A few points below are worth confirming or tightening before merge — mostly documentation clarity and one backward-compat edge case.
Minor notes (not inlined):
- No unit tests exercise
goFileOf's three branches or the config-to-enumswitch. Given the"fname"collision semantics and thepos > 0vspos >= 0asymmetry, a small table-driven test (top-level file, nested file, shared basename, leading-_file, extensionless file × each enum value) would cheaply lock in intended behavior. - The accepted values
"dir"/"fname"live only as inline string literals; defining them as named constants shared by the switch, the warning message, and the doc comment would avoid future drift.
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.
No description provided.