sunxiaojian commented on PR #5641: URL: https://github.com/apache/gravitino/pull/5641#issuecomment-2497025984
> There are still a couple of things to fix: > > * There are unnecessary checks if tags are null that are not needed, as the CLI library will detect if you pass no tags and give an error. "Error parsing command line: Missing argument for option:". Some of your error messages have no way of being displayed. > * The code adds multiple args more than the option and now that function name is misleading. Change to only have --tag to have multiple arguments. > * commands's handle methods don't follow the same format of the other commands, it is best to be consistent here. > * The outout of create and delete can be improved. When creating a single tag the output is "Tags xxx created" it should say tag not tags, same with delete. > * When removing tags there an extra comma in the output - "xxx removed tag tagA,tagB, now tagged with nothing" > * The tests tagWithMultiArgs only test the CLI internal library code so I'm not sure they have value, it would be better to create these tests in TestTagCommands.java using mocks. About 1, I think it still needs to be verified because there may be situations where the “—tag” is not specified， -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
