-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
You should be able to make this simply inherit from Exception. It should handle taking a string automatically as part of Exception.
-
-
I'd prefer a bit of cleanup with the comment. The "Nothing too fancy here" isn't really helpful from a user's point of view. The comment about the literal strings can just be a standard # comment before the tokenizer.
-
-
-
Much of this doc block is about the implementation and background of the class, rather than how it's used. The doc block should be more about the purpose of the class and how to use it. The rest should go in standard comments.
-
-
-
-
-
-
"If fails" should probably be something like "If the token is not an operator, then it's a variable and will be returned."
-
-
-
-
-
-
-
-
-
-
-
-
-
This sentence doesn't make much sense to me. Also, maybe this is a regional difference, but it should be "it" instead of "he". Same with other comments in this file.
-
-
No space after """. Same with other doc blocks. Also, "first," not "fist". Same in right(). I'm not really sure what the comment means though. Can you reword it?
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
Can you add a TODO here and file a bug to get both the docs written and a link to the docs from this help text?
-
-
-
-
These imports should be absolute. reviewboard.policy.expression. You can run into interesting problems otherwise if importing the same module using two separate relative paths.
-
-
-
This would be fine for a # comment, but not a doc block. Can you add a doc block explaining what this class does?
-
-
-
-
Not a huge fan of this. It makes it harder to grep for usage of functions and stuff. Maybe rename applies to policy_applies in applier.py.
Policy support backend
Review Request #970 — Created Aug. 17, 2009 and discarded
This patch includes policy support for ReviewBoard. It includes the expression parser, a simple admin interface and a single action as a proof of concept. This is a fully extensible parser, that already has a few operators to serve as an example and more operators can be used in the future. The parser is based on a technique described in the book Beautiful Code, and has the advantage of being fast and creating a syntactic tree that can be evaluated under different contexts, and the disadvantage of having poor error messages for syntactic errors. This can be improved in latter versions using heuristics. Actions should be checked through the helper function policy.applier.applies, conviniently imported under the name `policy_checks` in the example. That function was caching syntactic trees, but I couldn't get the cache invalidated, so I removed it. I still don't know wether we should be using a reference to a string in the Policy class or a hardcoded string for action names, but in order to simplify the development I opted for the latter. More actions will have to be mapped for this patch to be considered complete, and those will come in future updates to this review. Also, there should be enough semantic in the operators themselves that an builder is feasible for a limited number of expressions. Also, the manual should be automatically generated, so should a syntax helper on the side of the screen.
The parser was tested a lot. The rest was tested only in Safari.
