-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
About the merging of subcommands #96
Comments
I prefer the merging just like |
Could you please provide an example of the proposal? I'd like to see some actual design work. |
@q384566678 That is a PR. Do you have a design document? |
@stevvooe This picture is my overall idea, the above separation of the three functions into a whole function. This is not only convenient to use, but also to reduce the duplication of some functions. |
@q384566678 Ok, that is a start. However, it looks like this is just thrown together. Let's use an object verb pattern to structure the commands:
The above is just an example, but as you can see, we need to do some thinking about the objects that exist and the actions that can be taken with them. |
@stevvooe OK, I will rectify the next, thank you for your suggestion. |
@q384566678 Please don't make changes on this PR. Make a design document with a proposal. |
@stevvooe I don't quite understand what you said |
@q384566678 Write something that describes what you plan to do. It should include details about what commands and subcommands there are. |
@stevvooe Here's what I want to do: oci-image-tool [global options] command [command options] [arguments...] commands: global options: |
@q384566678 Let's try and organize around
However, I am still not sure what a bundle and what the difference between unpacking and creating is. I think opencontainers/image-spec#467 needs to be fleshed out further. |
@stevvooe I think the |
@q384566678 Its not about making it complicated. Its about making it reflect the actual problem. I am pretty familiar with this area and when I look at the current state of the image tool and the proposed state, I have no clue what it is trying to do. I would really love for someone to pick up opencontainers/image-spec#467 and start doing the engineering work of defining what these actions are. Without it, we are just shooting in the dark and hope stuff sticks to the wall. |
I think we might can submit a PR firstly, to combine the sub-packages under |
I have already submitted #103. |
Thanks, I see and am reading it. |
@xiekeyang @q384566678 As requested earlier, it would be best if we could see some up front design work on the command before going forward. Please see opencontainers/image-spec#588 (comment) for an example. That comment shows a command and its output. I think we really need to see this kind of detail tied to use cases before we can move forward with anything. Also, after taking a look at #103, it looks like it is introducing concepts that don't exist. For example, it talks about an "image file" and "image source layout". Neither of these are really defined by the specification. Without rigor here, we may end up introducing a component that should be specified but is not. For the most part, the image-tools should focus on packing and unpacking to a bundle and config. I think we can break it down into these use cases:
If we drive the design from these uses (and don't get distracted), I think we can ensure the tool will be much more aligned. There are probably other cases we need to consider, but let's focus on making these rock solid before adding functionality. Let me know if more clarification is needed. |
I totally agree with you not to add any functionality in this patch. I had suggested to just combine current tools as format To be honest I'm a little distraught when developing Do you think it is worthy to move this PR forward to what I suggest above, or we should end up this? |
@xiekeyang #103 still doesn't make the difference between |
@xiekeyang Perhaps, we go ahead and merge #103 and then follow up the problems with |
I do not understand why the image-tools in the three sub-command split into three software, I think these three commands should be combined into a tool, after all, they belong to image-tools, I think the merger will Easy to understand and application, if I was a user, I would think that the use of a software implementation of the three commands than the use of three software to implement a command to be more convenient.
I currently have a plan to merge, don't know how you think .
The text was updated successfully, but these errors were encountered: