I have been collecting list of issues at http://www.mediawiki.org/wiki/Git/Conversion#Open_issues_after_migration
I'd like start discussion how to solve these issues and in what time frame.
== Code review process == I would very much like to have the full patch diffs back in emails so that I can quickly scan for any i18n issues. Also Gerrit is just slow enough that I must either waste time waiting for it to load the interface, or do a context switch by reading next commit. Not to mention the need to open the diffs for each file in tab.
I can somewhat work with this right now by skipping commits unlikely to have anything relevant (though you never know, as not even the file names are mentioned), but when the number of commits pick up the speed it's not going to work.
== Emails == Related to above, I want to scan all (submitted?) changes for the issues. Currently there is no easy way to subscribe to changes of all repositories.
In theory I could do the same inside Gerrit, would it provide a easily navigatable list which records what I have looked at.
== Unicode issue in Gerrit == This must be fixed (dataloss). See https://gerrit.wikimedia.org/r/#change,3505 for example
== Local changes == How to handle permanent local changes? There have already been suggestions: * use git stash (not fun to do for every push) * use git review --no-rebase (no idea if this is a good idea) * commit them to local development branch (but then how to rebase changes-to-commit to master before pushing?)
== How to FIXME == We need a way to identify *merged* commits that need fixing. Those commits should be our collective burden to fix. It must not rely on the reporter of the issue fixing the issue him/herself or being persistent enough to get someone to fix it.
I was suggested to use bugzilla, but it's a bit tedious to use and doesn't as-is have the high visibility like FIXMES used to have.
-Niklas
Le 27/03/12 11:57, Niklas Laxström a écrit :
I have been collecting list of issues at http://www.mediawiki.org/wiki/Git/Conversion#Open_issues_after_migration
I'd like start discussion how to solve these issues and in what time frame.
I would really prefer that we add issues in the Bugzilla tracker. We already have enough mess to keep track of and a wiki has already proven how bad it is for issues tracking.
I am answering below but will not follow up. We should have bug opened for all issues potentially copy pasting from this mail and my answer.
== Code review process == I would very much like to have the full patch diffs back in emails so that I can quickly scan for any i18n issues. Also Gerrit is just slow enough that I must either waste time waiting for it to load the interface, or do a context switch by reading next commit. Not to mention the need to open the diffs for each file in tab.
<snip>
There are several issues in that paragraph.
Full diffs in emails:
I dislikes having full diffs in emails and they are lacking colors making them basically useless to me. That also makes mail processing slower and makes my inbox very huge. If we ever do that, please please make it a per user option.
Gerrit slowness:
I do not have that much an issues. Possible root causes could be: - you have a slow computer - your network connection is bad - Gerrit client interface is badly programmed - Gerrit server is overloaded Or that might just be because delay between click is not good enough for you. Maybe we could investigate on the server side or add a layer of caching in front of Gerrit. Anyway, something worth a bug so we could investigate it.
scan for i18n:
Can this be somehow scripted / made more automatic? We should probably eastablish a list of recurrent issues then have a script to automatically analyse files to find possible culprit. Two possibles examples comes to mind: - messages being added in MessagesEn.php and missing from Messages.inc. 100% sure this can be scripted - wfMessage() being used without an explicit formatting call (->parse, ->parseAsBlock(), ->text()). There is again 100% possibility to script that using PHP tokenizer.
diff files in each tab:
I am a huge fan of this feature, I find it way easier to handle than a large and long page. I have already written about it in wikitech-l some weeks ago. One possible workaround would be to add a link to the Gitweb full diff in Gerrit GUI.
This should probably be reported upstream.
== Emails == Related to above, I want to scan all (submitted?) changes for the issues. Currently there is no easy way to subscribe to changes of all repositories.
Indeed, you have to manually subscribe to get email notifications for each projects. Once subscribed you will always receive everything with no easy way to stop the mail spam even if you have no interest in a specific change. As I understand it, you are looking for something like the mailing list that received all mediawiki svn commits. We will have to look at Gerrit to have some mailing list to get notified of any patchset submitted to Core and Extension.
Need investigation and a report upstream.
In theory I could do the same inside Gerrit, would it provide a easily navigatable list which records what I have looked at.
One way is to use the Gerrit search box and look for 'mediawiki/' that will give you all changes ever submitted to mediawiki/* projects.
== Unicode issue in Gerrit == This must be fixed (dataloss). See https://gerrit.wikimedia.org/r/#change,3505 for example
See: https://bugzilla.wikimedia.org/35455
Upstream recommendation is to use the embed H2 database or a real database such as postgre.
== Local changes == How to handle permanent local changes? There have already been suggestions:
- use git stash (not fun to do for every push)
- use git review --no-rebase (no idea if this is a good idea)
- commit them to local development branch (but then how to rebase
changes-to-commit to master before pushing?)
I have talked about it with Niklas and have no idea how that could be fixed. Maybe by working in a local branch and then have the current commit to be merged to a clean master before submission. That would require some scripting.
Niklas, I guess you want to send a new mail to wikitech-l so it receives more attention.
== How to FIXME == We need a way to identify *merged* commits that need fixing. Those commits should be our collective burden to fix. It must not rely on the reporter of the issue fixing the issue him/herself or being persistent enough to get someone to fix it.
I was suggested to use bugzilla, but it's a bit tedious to use and doesn't as-is have the high visibility like FIXMES used to have.
We should have less fixme nowadays since we have adopted a pre merge review, it still happens from time to time though. Our bug report is https://bugzilla.wikimedia.org/35535
Thanks Niklas for your long feedback :-)
On 28/03/12 18:10, Antoine Musso wrote:
Gerrit slowness:
I do not have that much an issues. Possible root causes could be:
- you have a slow computer
- your network connection is bad
- Gerrit client interface is badly programmed
- Gerrit server is overloaded
Or that might just be because delay between click is not good enough for you. Maybe we could investigate on the server side or add a layer of caching in front of Gerrit. Anyway, something worth a bug so we could investigate it.
I also find gerrit delays annoying, so I went to https://gerrit.wikimedia.org/r/#change,3794 and opened the file diff. It turns out to be a well suited change for testing, since it's just a "test 1" -> "test 2".
The developer console showed:
[20:41:13.563] GET https://gerrit.wikimedia.org/r/gerrit/deferredjs/6B3633B5038AF814D44FB7828C3... [HTTP/1.1 304 Not Modified 183ms] [20:41:13.867] POST https://gerrit.wikimedia.org/r/gerrit/rpc/ChangeDetailService [HTTP/1.1 200 OK 966ms] [20:41:14.835] POST https://gerrit.wikimedia.org/r/gerrit/rpc/PatchDetailService [HTTP/1.1 200 OK 655ms]
That's a bit more than 1.8s, although it feels slower, maybe due to the later javascript and CSS processing of those messages.
I suspect the first one avoidable, since it seems to already include versioning in the url. The content could also be improved, I wouldn't be fond of the work of the used minimizer.
But, why does it need the other two slow AJAX messages? And just a one-line diff! :(
My round-trip time to manganese is ~164ms, so there's ample room for improvement.
Part of the gerrit slowness is that the web server is in eqiad and its database is in pmtpa. We noticed some slowness of the interface when moving the gerrit service to a new server. That said, the new server handles the load much better, so overall it was worth the extra latency.
We need to point the gerrit server at a database in eqiad at some point soon. I'll look into this when I get back from vacation.
On Sat, Mar 31, 2012 at 4:02 AM, Platonides Platonides@gmail.com wrote:
On 28/03/12 18:10, Antoine Musso wrote:
Gerrit slowness:
I do not have that much an issues. Possible root causes could be: - you have a slow computer - your network connection is bad - Gerrit client interface is badly programmed - Gerrit server is overloaded Or that might just be because delay between click is not good enough for you. Maybe we could investigate on the server side or add a layer of caching in front of Gerrit. Anyway, something worth a bug so we could investigate it.
I also find gerrit delays annoying, so I went to https://gerrit.wikimedia.org/r/#change,3794 and opened the file diff. It turns out to be a well suited change for testing, since it's just a "test 1" -> "test 2".
The developer console showed:
[20:41:13.563] GET https://gerrit.wikimedia.org/r/gerrit/deferredjs/6B3633B5038AF814D44FB7828C3... [HTTP/1.1 304 Not Modified 183ms] [20:41:13.867] POST https://gerrit.wikimedia.org/r/gerrit/rpc/ChangeDetailService [HTTP/1.1 200 OK 966ms] [20:41:14.835] POST https://gerrit.wikimedia.org/r/gerrit/rpc/PatchDetailService [HTTP/1.1 200 OK 655ms]
That's a bit more than 1.8s, although it feels slower, maybe due to the later javascript and CSS processing of those messages.
I suspect the first one avoidable, since it seems to already include versioning in the url. The content could also be improved, I wouldn't be fond of the work of the used minimizer.
But, why does it need the other two slow AJAX messages? And just a one-line diff! :(
My round-trip time to manganese is ~164ms, so there's ample room for improvement.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 28/03/12 18:10, Antoine Musso wrote:
scan for i18n:
Can this be somehow scripted / made more automatic? We should probably eastablish a list of recurrent issues then have a script to automatically analyse files to find possible culprit. Two possibles examples comes to mind:
- messages being added in MessagesEn.php and missing from
Messages.inc. 100% sure this can be scripted
- wfMessage() being used without an explicit formatting call (->parse,
->parseAsBlock(), ->text()). There is again 100% possibility to script that using PHP tokenizer.
How would you script that if you don't have the files? (as they are pending a merge) Could we have a branch which included all non-abandoned patches? Or maybe all patches whose total feedback is not negative.
== Local changes == How to handle permanent local changes? There have already been suggestions:
- use git stash (not fun to do for every push)
- use git review --no-rebase (no idea if this is a good idea)
- commit them to local development branch (but then how to rebase
changes-to-commit to master before pushing?)
I have talked about it with Niklas and have no idea how that could be fixed. Maybe by working in a local branch and then have the current commit to be merged to a clean master before submission. That would require some scripting.
Niklas, I guess you want to send a new mail to wikitech-l so it receives more attention.
Maybe by developing on a clean repository, from which you pull to the with-patches one?
== How to FIXME == We need a way to identify *merged* commits that need fixing. Those commits should be our collective burden to fix. It must not rely on the reporter of the issue fixing the issue him/herself or being persistent enough to get someone to fix it.
I was suggested to use bugzilla, but it's a bit tedious to use and doesn't as-is have the high visibility like FIXMES used to have.
We should have less fixme nowadays since we have adopted a pre merge review, it still happens from time to time though. Our bug report is https://bugzilla.wikimedia.org/35535
Can someone measure at CodeReview the number of revisions which went to fixme after having been on ok? gerrit system allowing pre-review doesn't help with the 'false review rate'. There *will* be bugs which get merged into the main repo. Not every master status will be perfectly stable, as we wish it were. Ability to mark the patchsets as fixme is a good tool. If we had a list of follow-ups in gerrit, that would also be useful.
On March 30 2012 at 21:25, Platonides wrote: <snip>
How would you script that if you don't have the files? (as they are pending a merge) Could we have a branch which included all non-abandoned patches? Or maybe all patches whose total feedback is not negative.
One just have to fetch the non-merged change from Gerrit. Git magic is:
# Fetch change 1234 $ git fetch gerrit refs/changes/34/1234
# Switch to that branch: $ git checkout FETCH_HEAD
Remember that "refs/changes/34/1234" is just a pointer to some commit object exactly like "master". So you can switch to it, run your tests and submit their results.
<snip>
We should have less fixme nowadays since we have adopted a pre merge review, <snip>
Can someone measure at CodeReview the number of revisions which went to fixme after having been on ok? gerrit system allowing pre-review doesn't help with the 'false review rate'. There *will* be bugs which get merged into the main repo. Not every master status will be perfectly stable, as we wish it were. Ability to mark the patchsets as fixme is a good tool. If we had a list of follow-ups in gerrit, that would also be useful.
We already have the fixme feature. That is done by down voting a patchset in the codereview field (the infamous : "I would prefer that you didn't submit this").
The follow up we have been abusing is also build in since you usually just send a second patch. A follow up to a previous merge is either a new feature or a bug fix, it can still reference the change number but I am not sure there is any value in doing so.
Whenever someone would need to use followup, he should probably use a branch instead. Branch are cheap, use them :-]
On 31 March 2012 10:56, Antoine Musso hashar+wmf@free.fr wrote:
We already have the fixme feature. That is done by down voting a patchset in the codereview field (the infamous : "I would prefer that you didn't submit this").
No that isn't fixme. Fixme means "something needs fixing in this commit which is already merged". This could also be solved with the free form tagging feature. Unfortunately it has been sitting in upstream bugzilla for three years without any activity. Can we really afford to wait if/when they are going to implement it?
The follow up we have been abusing is also build in since you usually just send a second patch. A follow up to a previous merge is either a new feature or a bug fix, it can still reference the change number but I am not sure there is any value in doing so.
There is huge benefit knowing what subsequent fixes/changes has been done to the commit you are currently looking at. Gerrit will reduce the amount of follow-ups, but they will still exist. -Niklas
On 31/03/12 09:56, Antoine Musso wrote:
On March 30 2012 at 21:25, Platonides wrote:
<snip> > How would you script that if you don't have the files? (as they are > pending a merge) > Could we have a branch which included all non-abandoned patches? Or > maybe all patches whose total feedback is not negative.
One just have to fetch the non-merged change from Gerrit. Git magic is:
# Fetch change 1234 $ git fetch gerrit refs/changes/34/1234
# Switch to that branch: $ git checkout FETCH_HEAD
Remember that "refs/changes/34/1234" is just a pointer to some commit object exactly like "master". So you can switch to it, run your tests and submit their results.
And how do I know that what I need is refs/changes/34/1234 ? Do you have a way of listing all changes of a certain repository?
<snip> >> We should have less fixme nowadays since we have adopted a pre merge >> review, <snip> > > Can someone measure at CodeReview the number of revisions which went to > fixme after having been on ok? gerrit system allowing pre-review doesn't > help with the 'false review rate'. > There *will* be bugs which get merged into the main repo. Not every > master status will be perfectly stable, as we wish it were. > Ability to mark the patchsets as fixme is a good tool. If we had a list > of follow-ups in gerrit, that would also be useful.
We already have the fixme feature. That is done by down voting a patchset in the codereview field (the infamous : "I would prefer that you didn't submit this").
The follow up we have been abusing is also build in since you usually just send a second patch. A follow up to a previous merge is either a new feature or a bug fix, it can still reference the change number but I am not sure there is any value in doing so.
Suppose A created a foo module in r12345, this was reviewed by B.
With the old system, if I perform a change: Don't leak passwords in the foo module. Follow-up r12345.
Then both A and B get alerted of that change, it is listed in the page, when C goes to cherry-pick the foo module to production, he notices it immediatly and also includes the follow-up.
Whenever someone would need to use followup, he should probably use a branch instead. Branch are cheap, use them :-]
This makes no sense. You're probably thinking on a feature made of revisions linked with follow-ups, which should have been a branch in svn. Note however that not having a state with all the code, you don't notice the changes. And if you don't see them until they are merged, you're back with the same intermingled revisions.
wikitech-l@lists.wikimedia.org