justinmclean commented on PR #5641: URL: https://github.com/apache/gravitino/pull/5641#issuecomment-2496445689
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 multip more than the option --tag to have multiple arguments, it should only change tags. - 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 a tag there an extra comma in the output - "xxx removed tag tagA, 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. -- 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]
