-
-
/trunk/reviewboard/reviews/models.py (Diff revision 1) I for got to take out my debut messages. I will remove them in the morning
If the summary is more than 300 chars Trim it down
Review Request #482 — Created July 31, 2008 and submitted
| Information | |
|---|---|
| JSlominski | |
| Review Board SVN (deprecated) | |
| 515 | |
| Reviewers | |
| reviewboard | |
When using post-review with perforce there is an error if the CL description has more that 300 chars without a newline. A SQL warning is generated (truncating summary) that will cause post0review to throw an error 500. There is an example of the error page in Bug 515. This is my first attempt in Python so the code may not be the best. I created a function that will truncate a string at the first '.' or after the 300th char.
I submitted and updated a review that had more than 300 chars without a \n and a '.'. I submitted and updated a review that had more than 300 chars without a \n and did not have a '.' I submitted and updated a review that had more than 300 chars without a \n and had '.' after the 300th char
JS
-
I started to review this but then I began to think about what we want to do here. I'm not sure truncating like this is really the desired behavior. We should be limiting the size in the web UI (which I believe we do) and we should probably provide a validation error to clients using the web API that set a value that's too long. It's then up to the calling program to limit the size. So the truncation logic should probably go in post-review.
-
-
-
/trunk/reviewboard/reviews/models.py (Diff revision 1) Trailing whitespace. Also in other places in the change.
-
-
/trunk/reviewboard/reviews/models.py (Diff revision 2) This algorithm isn't quite right. Two problems with it: 1. If the first sentence is longer than 300 characters, it'll still leave a string too long. 2. It would be nice to include as many sentences as possible within the allowed 300 characters instead of just the first.
-
-
-
-
/trunk/reviewboard/reviews/models.py (Diff revision 3) This is way more complicated than it needs to be. Here's what I suggest: def truncate(string, n): if len(string) > n: string = string[0:n] i = string.rfind('.') if i != -1: string = string[0:i + 1] return string
CF
-
-
/trunk/reviewboard/reviews/models.py (Diff revision 3) Is is possible to pull the number 300 from a config file? Would be nice to avoid magic numbers in the code... just my $0.02
CF
-
-
/trunk/reviewboard/reviews/models.py (Diff revision 3) No doubt, obviously not trying to start a flame-war or nit-pick session. Commenting is a great idea, especially if you factor out the 300 into a global or class variable like "max_summary_length": /* truncate summary to avoid p4 error - experimental limit is 300 chars */ self.summary = truncate(self.summary, max_summary_length)