search

mozilla / newnewtab

New Tab page: content server and recommendation service
https://wiki.mozilla.org/Apps/newnewtab
6 stars 5 forks source link

Closes #27: Whitelist app for localstorage usage? #46

Closed mzhilyaev closed 12 years ago

mzhilyaev commented 12 years ago

Added permissions to access AppCache

tofumatt commented 12 years ago

Looks good, but for next time: you should only need "Close[s] #27" to mark that issue as closed; putting the whole issue name in the commit looks odd.

Mardak commented 12 years ago

It's standard practice in mozilla-central to include the bug summary with the bug number in commit messages, see mozilla-central: http://hg.mozilla.org/mozilla-central/

It's useful when looking through the git logs when not using github to at least get a sense of what was being fixed in addition to the other part of the commit summary that describes the change.

Mardak commented 12 years ago

But if the style here is to not include the bug summary, then I guess that's fine to leave it out and just describe the fix on the first line instead of the second line. Any preference on casing and punctuation of the commit message?

Mardak commented 12 years ago

Although in this case, the bug summary isn't that useful, so I usually edit the summary. "Whitelist app for localstorage usage?" doesn't really describe the fix with the question mark.

fwenzel commented 12 years ago

Yup, I would write:

"Whitelist app for localstorage. Fix Issue #27."