On 07/19/11 11:55, Danek Duvall wrote:
Shawn Walker wrote:
On 07/19/11 09:49, Danek Duvall wrote:
...
signature.py:
- line 478: What about this code matches the comment? Or is the comment
just a warning of some sort?
It's my attempt to document that "value" is checked for in validate()
even though it's a key attribute. In all other cases, we require the
key attribute at action.__init__(), but this case requires a special
exception because of how signing uses signature actions.
Okay. I guess I don't understand why the comment says (as far as I
understand it) "you don't have to have a value attribute during
publication" and then proceeds to emit an error if you don't have it
without checking if we're in publication.
I don't have that reading at all of it, it says "...this can only be
required during publication" and the docstring for validate() makes it
clear the function checks things required during publication.
However, I've completely reworded the comment, so perhaps it will be
clearer this time.
- line 481: 'if "value" not in self.attrs'?
Because you can specify value="" on an action and that's not enough.
But regardless, I've switched this over to using required_attrs on
_validate().
If you want to enforce a non-empty value attribute specially, that's fine,
but I'd guess that it actually needs more validation than that in the end.
Right, I'm just going for non-empty as a sanity check.
dependencies.py:
- line 1352: why treat this as a keyword arg?
Because it is?
Everywhere else, we treat it as an optional positional argument. In fact,
one of the places where you added the argument you didn't bother with the
keyword.
It's defined in the class as a keyword argument, but yes, we
inconsistently apply use of this everywhere. Of course, the real
question remains why we even bother with build_release at all...
...
t_pkgsend.py:
- line 118: what's wrong with this manifest?
Nothing is wrong with the manifest, but originally, testing this
required providing a manifest as an argument. I've removed the
manifest bit now.
So what causes pkgsend to die here? Is it the lack of a repo to publish
to?
Yes, which should be clearer now that I've removed the manifest bits so
it's closer to the comment that's trying to indicate that is what the
test is for.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
