Closed (fixed)
Project:
Project issue tracking
Version:
6.x-1.x-dev
Component:
Documentation
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
20 Jan 2009 at 19:24 UTC
Updated:
9 Feb 2009 at 12:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
mr.baileysPatch attached. The only field I was unsure about is {project_issue_state}.author_has.
Comment #2
aclight commentedThat means whether or not the author of an issue has the permission to set the state of an issue to a certain value. For example, you might have a system where users can create issues to report bugs but wouldn't normally have the permission to change the status (that would be the developers job). But you might want the original creator of an issue to be able to close it, if for example he realized that he reported a bug that wasn't really a bug in the first place.
Some other comments:
+ 'description' => t("Table containing issue-related settings per project."),I'd go with+ 'description' => t("Table containing per-project issue-related settings."),You're being inconsistent with "email" vs "e-mail" (see #363937: Use "e-mail" in all user facing strings). Also, at a quick glance it might not be obvious that the email mentioned here is the one sent to {project_projects}.mail_copy and not the emails sent to subscribers. So you might use this instead:
Both of these comments also apply to the next field,
I'd instead go with
Likewise, the same change would apply to
... project_release nodes. No underscore between release and nodes.
I think "... original metadata values ..." might make this a bit more clear.
Maybe this is better
Maybe
By the way, I believe the coding standards specify that whenever possible, single quotes should be used for strings instead of double quotes. My understanding is that there are no longer any real performance reasons to do this, but I could be mistaken. dww and hunmonk are usually pretty strict about following the coding standards, but on this issue I am not sure if they really care.
Comment #3
dwwA) I added the blurb for "author_has":
t('Boolean indicating whether or not the author of an issue may use this issue status in that issue, regardless of the permissions controlling site-wide use of this status.')
(that's a little clumsy, further edits would be welcome). ;)
B) I converted t("...") to t('...') since that's the standard unless you need "" for some reason.
C) Made a few minor edits for clarity.
Committed to HEAD.
Thanks!
-Derek
Comment #4
mr.baileysThanks for the detailed review & feedback, very much appreciated! Based on the feedback, I made some additional changes to HEAD (patch attached):
1,2 & 3) fixed inconsistent spelling of "e-mail" & improvements to wording.
4) Actually, the "References project_releases node"-part can be dropped altogether since the description starts with "The {project_release_nodes}.rid (version identifier) for this issue...". Removed it from the description
5) changed description to read "...original metadata values..."
6) I agree that "...after this comment was made." sounds a bit akward. Not sure about your suggestion
though: although correct from a code-perspective, from a data-perspective the title could have remained unchanged. I'd like to have this description convey that the comment might have changed the title of the issue, and that this title in turn could have been changed again by further comments to this issue. Unfortunately, I can't seem to come up with a decent way to phrase this... I would also use "this comment" instead of "a comment".
7) Yes, that does sound better, changed it.
Regarding quotes and coding standards: my initial changes had some single-quotes in the descriptions, and since the coding standards mentioned the following exception to the "single-quotes preferred"-rule
I'd converted all descriptions to double-quotes. The single-quotes in the descriptions have since been removed though, so I'm ok with the single quotes wrapping the descriptions.
Comment #5
aclight commentedWell, the point I was trying to make was that whether or not the user actually changes the value of any of the metadata in a comment, it is all stored in the database just as if the user had changed the values. I don't really like my proposed phrasing that much better than yours, and if neither of us can come up with something better it's probably not worth spending more time on it, since this is an extremely minor point. I just wanted to bring it up previously in case you were able to come up with something snappy :)
Comment #6
dww@aclight #2: wow, sorry, I totally missed that you had commented here while I was reviewing and committing #1... sorry about that! All good points, glad they're now fixed. ;)
@mr.baileys: Yes, I'd normally just use " all the time for t() (to avoid the t('ain\'t this a shame') problem) if He-who-must-not-be-named and others didn't come screaming in my issue queue about it. :/
Thanks to both of you for your help here, committed #4 to HEAD.