-
Notifications
You must be signed in to change notification settings - Fork 18
Description
Description
Our validation command currently does not check if the flag keys in the manifest are unique. If we implement that there, it will be checked across all cli commands.
Context from PR
From #191 discussion:
Do we need to handle the case where the flag being deleted exists more than once in the manifest? Or is that impossible?
It is handled in our add command:
cli/internal/cmd/manifest_add.go
Line 167 in b255228
| return fmt.Errorf("flag '%s' already exists in the manifest", flagName) |
However, it might be worth adding it to our Validate func. That's used by manifest.LoadFlagSet, which is called by all of our commands to load the flag set.
cli/internal/manifest/validate.go
Line 18 in b255228
| func Validate(data []byte) ([]ValidationError, error) { |
https://github.com/search?q=repo%3Aopen-feature%2Fcli%20manifest.LoadFlagSet&type=code
This way, if someone adds the flag manually by editing the json for instance, the next usage of the cli against the manifest would throw invalidation errors immediately.
I would support that, but suggest in another PR for hygiene.
Originally posted by @kriscoleman in #191 (comment)