abderrahim commented on code in PR #2030:
URL: https://github.com/apache/buildstream/pull/2030#discussion_r2183331511


##########
src/buildstream/_frontend/cli.py:
##########
@@ -552,7 +555,29 @@ def build(
             _PipelineSelection.ALL,
         ],
     ),
-    help="The dependencies to show",
+    help="Types of Elements to Show",
+)
[email protected](
+    "--kind",
+    "-k",
+    default=[],
+    show_default=True,
+    multiple=True,
+    type=FastEnumType(
+        _ElementKind,

Review Comment:
   "Element Kind" is definitely not how you should name this. The element kind 
is already a concept in buildstream, and it's not this.
   
   I don't know what would be the best way to name this option though. info? 
keys?
   
   Let's wait for other maintainer's opinions, they might have a better idea.



##########
src/buildstream/_frontend/cli.py:
##########
@@ -538,6 +540,7 @@ def build(
 @click.option(
     "--except", "except_", multiple=True, type=click.Path(readable=False), 
help="Except certain dependencies"
 )
[email protected]("--json", "as_json", is_flag=True, help="Output the information 
as machine readable JSON")

Review Comment:
   It would be nice to have a similar option that outputs YAML, as that's what 
buildstream uses.



##########
src/buildstream/_frontend/cli.py:
##########
@@ -552,7 +555,29 @@ def build(
             _PipelineSelection.ALL,
         ],
     ),
-    help="The dependencies to show",
+    help="Types of Elements to Show",

Review Comment:
   Please keep the old description. "Types of elements" doesn't make sense here.



##########
src/buildstream/_frontend/cli.py:
##########
@@ -629,19 +654,44 @@ def show(app, elements, deps, except_, order, format_):
     \b
         bst show target.bst --format \\
             $'---------- %{name} ----------\\n%{vars}'
+
+    **JSON OUTPUT**
+
+    The ``--json`` flag will cause bst to output information in machine 
readable
+    JSON. When using this flag you may also specify ``--kind`` to control the
+    type of information that is output.
+
+    To dump everything:
+    \b
+        bst show --json --kind all
     """
     with app.initialized():
 
+        if as_json and format_:
+            raise AppError("--format and --json are mutually exclusive")
+
+        if format_ and kind:
+            raise AppError("--format does not support --kind")

Review Comment:
   Not sure I like this error message, as I see `--format` and `--kind` (in 
your current implementation) to be essentially the same thing except `--format` 
is used in the default mode and `--kind` is used in the json mode.
   
   Maybe we could change it such that `--format` could take the special value 
`json`, and that would unlock the (replacement for the) `--kind` option.
   
   Let's wait for other opinions.



-- 
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]

Reply via email to