refactor: flag settings to be shared in option package

[JeyJeyGao]

Refactor:

• refactored the shared flag options to be in option package

Docs:

• improved specs/commandline/key.md

Validation:

• compared all command help docs with main branch

Resolve part of #1151

[codecov]

Codecov Report

Attention: Patch coverage is 95.67568% with 8 lines in your changes missing coverage. Please review.

Project coverage is 75.95%. Comparing base (ba4c018) to head (f390e08).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/notation/key.go	69.23%	3 Missing and 1 partial ⚠️
cmd/notation/login.go	50.00%	2 Missing ⚠️
cmd/notation/logout.go	50.00%	1 Missing ⚠️
cmd/notation/registry.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1187      +/-   ##
==========================================
- Coverage   76.08%   75.95%   -0.13%     
==========================================
  Files          71       78       +7     
  Lines        3625     3594      -31     
==========================================
- Hits         2758     2730      -28     
+ Misses        670      667       -3     
  Partials      197      197              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Two-Hearts]

This PR depends on #1192 due to the --allow-referrers-api flag.

[JeyJeyGao]

This PR depends on #1192 due to the --allow-referrers-api flag.

Updated.

[Two-Hearts]

After going through this 49-file-change PR, the very first question pop up is do we really need it... I'm not seeing a clear advantage of this refactoring over the original code base though.
The original structure has a clear internal/cmd/flags.go file containing the flags of Notation. But after the refactor, it's gone. In other words, it reduces the readability of the code base.
If I were new to Notation and would like to contribute on it, adding a flag may already bring in challenge. Originally, I could just simply add it into flags.go, now I have to figure out where to put it.

[JeyJeyGao]

After going through this 49-file-change PR, the very first question pop up is do we really need it... I'm not seeing a clear advantage of this refactoring over the original code base though. The original structure has a clear internal/cmd/flags.go file containing the flags of Notation. But after the refactor, it's gone. In other words, it reduces the readability of the code base. If I were new to Notation and would like to contribute on it, adding a flag may already bring in challenge. Originally, I could just simply add it into flags.go, now I have to figure out where to put it.

@Two-Hearts It is true that adding a pattern will slightly increase the learning curve, but for long-term maintenance, learning a pattern is beneficial. I guess you think the original code is more readable because we have worked on the codebase for a while. We are doing the refactoring for future extensibility. In this PR, I grouped the flags into option types. Each option can contain multiple flags to be shared across multiple commands. Each option can have its own methods with processing logic for those flags. For example, Format includes the logic to set the default format and supported formats. Timestamp supports the Validate logic, which was written multiple times before in both blob sign and oci sign. This means that each option can encapsulate its logic in its own type, avoiding the spread of code in other places.

When adding a new flag that is only used in a single command, we can directly set the flag in the file that defines the command. If it is a shared flag used in multiple commands, you can create an option under the option package. Compared to the original internal/cmd/flags.go, which is a single file that includes all the flags, the new approach is more extensible. The flags in the original file are not types, so they cannot have methods for themselves. Also, putting all flags in a single file is not extensible as the file will become larger and larger.

[Two-Hearts]

After going through this 49-file-change PR, the very first question pop up is do we really need it... I'm not seeing a clear advantage of this refactoring over the original code base though. The original structure has a clear internal/cmd/flags.go file containing the flags of Notation. But after the refactor, it's gone. In other words, it reduces the readability of the code base. If I were new to Notation and would like to contribute on it, adding a flag may already bring in challenge. Originally, I could just simply add it into flags.go, now I have to figure out where to put it.

@Two-Hearts It is true that adding a pattern will slightly increase the learning curve, but for long-term maintenance, learning a pattern is beneficial. I guess you think the original code is more readable because we have worked on the codebase for a while. We are doing the refactoring for future extensibility. In this PR, I grouped the flags into option types. Each option can contain multiple flags to be shared across multiple commands. Each option can have its own methods with processing logic for those flags. For example, Format includes the logic to set the default format and supported formats. Timestamp supports the Validate logic, which was written multiple times before in both blob sign and oci sign. This means that each option can encapsulate its logic in its own type, avoiding the spread of code in other places.

When adding a new flag that is only used in a single command, we can directly set the flag in the file that defines the command. If it is a shared flag used in multiple commands, you can create an option under the option package. Compared to the original internal/cmd/flags.go, which is a single file that includes all the flags, the new approach is more extensible. The flags in the original file are not types, so they cannot have methods for themselves. Also, putting all flags in a single file is not extensible as the file will become larger and larger.

I'm not convinced with the above detailed explanation, if you have to write an essay to explain the change, I'd say it already proves my concern. Let's not make a simple and stable code base more complicated.