labath added inline comments.

================
Comment at: packages/Python/lldbsuite/test/lldbtest.py:518
@@ +517,3 @@
+        if hasattr(func, "categories"):
+            cat.extend(func.categories)
+        func.categories = cat
----------------
tberghammer wrote:
> tfiala wrote:
> > This code could potentially accept a single category and drop the need for 
> > the list.  It could check the type of the input (i.e. the categories input) 
> > and, if a single element rather than a list, it could just cat.append() it.
> > 
> > That would enable a single argument passed to @add_test_categories()
> > 
> > Totally minor and you can skip this, it just makes for a nice usability 
> > enhancement provided the decorator is documented as such.
> +1: I think we will (accidentally) call the function by a single string in 
> some case so accepting it would be a good idea.
The test_categories.validate will catch the case where you forget to put the 
braces and throw errors at you, so it's not possible to get unexpected behavior 
here.

I may be old-fashioned, but I like when things have a single type, which is 
obvious from the code. Look at all the cases where we do `callable(bugnumber)` 
and tell me if it's obvious why a "bug number" should be "callable". Plus, none 
of our other decorators taking lists have this kind of magic in them (imagine 
how would this complicate the logic of expectedFailureAll).

If we want to provide a simpler syntax for adding a single category, i would go 
for adding a separate `@add_test_category` decorator for that.


http://reviews.llvm.org/D15451



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to