Daniel Kinzler <daniel(a)brightbyte.de> wrote: It works in Git, for example with "git log --grep". I do think that fixing typos should be preferred to preserving typos for eternity, and -1 is better than nothing, but up- loading a new changeset is how it should be done. I cannot follow that line of thought at all. The "tag line" is rather short and should give the reader a summary of what the commit is about when browsing through a list of commits. Reserving part of that for a bug number would only be useful if the reader could associate the underlying issue by the number, but apart from bug #1 and a few (other) tracking bugs noone will be able to do so, and those bugs will (un- fortunately) never be closed. Is there another software project that uses the summary line in a similar way to MediaWiki? Tim

(anonymous) wrote: This should be "Fixes-Bug:" or something similar (I'll leave it to the native speakers whether "Bug-Fix:", "Fixed-Bug:", or something else is best at corresponding to "Change-Id:"). Tim

Tyler Romeo <tylerromeo(a)gmail.com> wrote: Then why would you use "Fixes: 123" if the commit doesn't actually fix bug #123? Tim

On Wed, Jan 23, 2013 at 1:17 PM, Tyler Romeo <tylerromeo(a)gmail.com> wrote: It's even easier than that: https://gerrit.wikimedia.org/r/#/q/bug:43523,n,z Gerrit indexes these values, so you can search for bug:123 or rt:456. -Chad

Can't we just use Gerrit's builtin topics for that? *--* *Tyler Romeo* Stevens Institute of Technology, Class of 2015 Major in Computer Science www.whizkidztech.com | tylerromeo(a)gmail.com On Wed, Jan 23, 2013 at 2:28 PM, Tim Landscheidt <tim(a)tim-landscheidt.de>wrote;wrote: > Tyler Romeo <tylerromeo(a)gmail.com> wrote: > > > That's my point. You wouldn't use "Fixes: 123" if it doesn't actually fix > > bug 123. However, what if there's a case like I said, where a bug is > fixed > > across multiple commits. If you're using the "Fixes" tag, then > technically > > you should only be tagging the last commit that finally fixes the bug, in > > which case all the other commits are left unmarked and are lost in the > > repository. With just a "Bug" tag, it indicates that the commit is > related > > to the bug. > > Then use "Relates-To:" for the relationships, and "Fixes:" > for the fixes. > > > My reasoning has to do with the motivation behind why we tag commits. > Maybe > > I'm wrong, but the reason we tag commits with bug numbers is so that, in > > the future, if one wants to find the commit(s) that fixed a certain bug, > > they can do a quick grep search on the commit log and find the relevant > > commits. > > > [...] > > I don't agree with that. There are lots of bugs that get > fixed unknowingly because most of the 5034 bugs are not on > the developers' radar. So any (consistent) information on > which commit has fixed a bug must be kept in Bugzilla as we > can't change Git's history, and it's hard to imagine a case > where you want to know which commit(s) fixed bug x without > looking at Bugzilla's page on bug x. > > Tim > > > _______________________________________________ > Wikitech-l mailing list > Wikitech-l(a)lists.wikimedia.org > https://lists.wikimedia.org/mailman/listinfo/wikitech-l >

On 01/15/2013 06:58 AM, Nikola Smolenski wrote: I agree. Well said. A minor mistake like that that doesn't affect grepping isn't worth a -1, since it doesn't affect the readability of the code itself. Matt Flaschen

----- Original Message ----- Talk to the people over on the derivative wikitext list, spun off to contain discussions relevant to creating a replacement MWText parser (there are, I think 5 or 6 projects in varying degrees of activity; though the list itself is pretty quiet). Cheers, -- jra -- Jay R. Ashworth Baylink jra(a)baylink.com Designer The Things I Think RFC 2100 Ashworth & Associates http://baylink.pitas.com 2000 Land Rover DII St Petersburg FL USA #natog +1 727 647 1274

Its not that difficult to read through the text before you commit, right? At least try to remove the most obvious spelling errors. Perhaps I just find to much BS I should have given a -1 or even a -2 in some cases. John

I agree with Antoine. Commit messages are part of the permanent history of this project. From now until MediaWiki doesn't exist anymore, anybody can come and look at the change history and the commit messages that go with them. Now you might ask what the possibility is of somebody ever coming across a single commit message that has a typo in it, but when you're using git-blame, git-bisect, or other similar tools, it's very possible. Also, a -1 is not a -2. It won't block the change from being merged; it's just a notification that something needs to be fixed, however minor. I'm still in favor of requiring every tag line to contain either (bug I'm not so sure about *every* commit, but I definitely agree that this needs to be enforced more. If you're fixing something or adding a new feature, there should be a bug to go with it. *--* *Tyler Romeo* Stevens Institute of Technology, Class of 2015 Major in Computer Science www.whizkidztech.com | tylerromeo(a)gmail.com On Tue, Jan 15, 2013 at 8:37 AM, Antoine Musso <hashar+wmf(a)free.fr> wrote:

-- - Brian Caution: The mass of this product contains the energy equivalent of 85 million tons of TNT per net ounce of weight. On Tue, Jan 15, 2013 at 10:57 AM, Daniel Kinzler <daniel(a)brightbyte.de> wrote: On the other hand, new users may be attracted to the fact that we have high standards. I agree that spelling is a valid reason for a -1. After all, -1 is not the same as a revert in the svn days, it simply means that the commit is not yet "perfect". (Even in svn a revert was supposed to be no big deal). -bawolff

Well, I would prefer to get a notice that I made a typo than having that embarrassing typo in the commit log forever. That's the point of using a gating system, right? :) So yes, I do think they should be corrected. (And I have committed typos in both commit messages and inside files, just as anyone else)

On Wed, Jan 16, 2013 at 7:09 AM, Tim Starling <tstarling(a)wikimedia.org> wrote: The motivation factor works with the two positions. I felt a little demotivated after having read all these "we don't care about typos" positions at the start of the thread and felt really relieved to read this is not the consensus opinion. Especially, as, in September and October, when I weren't at ease with my -1 typo reviews, I mailed four people I reviewed their change such a way to ask them what they prefer: a -1 review or a new patchset fixing the typo directly (I weren't at ease with the idea to resubmit a patchset without prior author consent either). Three of them didn't bother to answer me at all (for the anecdote, the 4th preferred a -1 review). So we quit the realm of "the workflow and the UI don't allow easy correction" to enter into the realm of "we don't care". On a side matter, typos could be a symptom of another issue: how important a commit message is? Should it be a formality to expedite in 30 seconds or an informative valuable text describing the change, crafted with care and proofread before submission or merge? What's the goal of a commit message as a changelog, communication tool and change documentation value? At the end, the direct commit message edit in the UI will offer an acceptable solution: corrections will be more trivial than found again my branch, amend the commit, resubmit as a new patchset. Meanwhile, we can suffer the last weeks of extra work for review spelling (as 0 or -1) purpose pending the Gerrit migration. And if you don't want to fix yourselves your commit, please create a list stating so on mediawiki.org, that will be a clear message for the code reviewers: "If you see a typo, would you be so kind as to fix it yourself and submit a new patchset?". -- Best Regards, Sébastien Santoro aka Dereckson http://www.dereckson.be/

On 17/01/13 00:14, Chad wrote: I didn't mention commit summaries in my post. My interest is in nitpicking in general. Jeroen calls arguments over commit summaries the /ultimate/ bikeshed, which they may or may not be; there are plenty of other examples which may compete for that title. Nitpicking is the minor end of the negative feedback spectrum. By definition, it has the smallest concrete payoff when advice is followed, in exchange for complex, context-dependent social costs. You should think carefully before you do it. -- Tim Starling

On 2013-01-16 7:20 PM, "Chad" <innocentkiller(a)gmail.com> wrote: wrote: If we're talking nitpicks in general. Ive seen -1 for things like someFunc($a, $b) instead of someFunc( $a, $b ) which I agree does more harm than good. I imagine how much someone considers spelling issues to be a minor "nitpick" varries quite a lot between people. -bawolff

Le 16/01/13 14:06, Sébastien Santoro a écrit : To me the commit summary introduce the patch to the reviewers and should also serve as a base for later debugging. It is always good to explain there the problem you attempt to solve or the feature being implemented, then explain your design choice and finally explain what you have tested / what people should be careful about. Of course I do not apply that principle to myself :-] Some of the repositories on which I am the sole contributor and self reviewing, have very short commit message. But I do try to add extensive comments in the code. A random example: We had a trivial configuration issue in our php.ini file. The apc.shm_size parameter was missing a suffix to the amount of memory that need to be added. The patch is: files/php/apc.ini -apc.shm_size=240 +apc.shm_size=240M I could have written: 'added M to apc mem size' Instead I wrote: ------------------------ php.ini apc.shm_size requires The apc.shm_size is an integer determining the size of each memory segment in MB. When missing the M suffix, PHP send us a warning in syslog: php: PHP Warning: PHP Startup: apc.shm_size now uses M/G suffixes, please update your ini files in Unknown on line 0 Upstream documentation: http://php.net/manual/en/apc.configuration.php#ini.apc.shm-size Having a change like 'fix preg call' is a -1 for me. I don't even bother reading the code in such a case. ------------------------ Yeah there are probably typo above, but at least that: - explain the change (I add M to apc.shm_size) - expose the issue (there is an annoying PHP warning) - reference doc for easy lookup for the reviewer Done. The PHP code base has usually very short summaries cause they simply reference the bug report. Example: Bug #62593 Added test for change The git project usually has very nice commit message. Have a look at http://repo.or.cz/w/git.git/commit/b72a1904aefb9e27014a11790f3af4dc90b38e8d That optimize some code path, the summary show up command line output before and after the patch :-] So yeah, we need extensive commit message. If someone don't bother fixing a typo (which is like 1 minute of work), I will not bother doing it for them nor approving their change. My time is better invested in reviewing another patch. -- Antoine "hashar" Musso

This is a ridiculous conversation and I can't believe it now spans +20 messages. ___Don't___ -1 a patchset for a typo. The result of that is far more catastrophic. We put off volunteers and people end up wasting valuable time doing rebases due to the time taken to find another code review. Code review is hard enough as it is. Make a ___suggestion___ about fixing the typo so the person who ends up merging it can do it themselves if necessary. +1 or -1 depending on the __code quality__ alone. Can we now return our attention to engaging in more interesting and useful conversations to the future of MediaWiki such as "[Wikitech-l] Disambiguation features: Do they belong in core or in an extension?" On Tue, Jan 15, 2013 at 10:09 PM, Tim Starling <tstarling(a)wikimedia.org> wrote: -- Jon Robson http://jonrobson.me.uk @rakugojon

I don't mind getting dinged for typos. If I'm being sloppy it's fair to point it out. However, I think the social contract should be that after I fix the typos you requested then you owe me a real code review where you look at the merits of the code. Code review is an awesomely useful but time consuming thing to provide. Patrolling for typos is not. Put it this way, if I concede and agree that the bikeshed can be green, the people who spent three hours arguing for green should feel obligated to turn up to the working bee to help with the painting. Luke Welling On Wed, Jan 16, 2013 at 10:40 AM, Bryan Tong Minh <bryan.tongminh(a)gmail.com>wrote;wrote:

considered be +2