584836 - hg should generate a tbpl consumable json pushlog

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Arpad Borsos [:Swatinem]]

view-source:http://hg.mozilla.org/mozilla-central/json-pushes is a really good start, all it needs is the changeset details from view-source:http://hg.mozilla.org/mozilla-central/json-info?node=fb2120e5e3a2cc07db3253d94ac1a41861e89c81

Comment 1

[(not currently active) Ted Mielczarek]

As I mentioned in IRC, we want to keep the default json-pushes with the current data, but you could add a query parameter, like ?full=1 that adds the changeset info. (The current json-pushes is very fast as it doesn't have to consult the Hg repo, just the sqlite db.)

Comment 2

[Arpad Borsos [:Swatinem]]

Attachment: patch

This was pretty straightforward.

Comment 3

[Axel Hecht [:Pike]]

Comment on attachment 463276 [details] [diff] [review]
patch

The releng folks would need the files, too, for bug 498055.

Not sure that I'm fond of having the changesets change sematics, but I don't have a recommendation that'd be backwards compat, either.

Comment 4

[Arpad Borsos [:Swatinem]]

Attachment: patch with files [pushed: comment 7]

Fair enough. Usually the number of changed files is quite manageable, I don't really know how the json size will look like for mega-merges like jm-fatvals.

Comment 5

[Arpad Borsos [:Swatinem]]

Attachment: make tbpl use the new pushlog json

This is completely untested as long as the pushlog changes aren't deployed yet, but it should work, hopefully.
This also gets rid of pipePushlog.php, yay.

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 463302 [details] [diff] [review]
patch with files [pushed: comment 7]

I agree with Pike that it's unfortunate that /json-pushes and /json-pushes?full=1 aren't producing compatible output. I guess it's not terrible, though, because you can always check typeof(push["changesets"][i]) == "string" to handle both cases.

After you land this you'll need to file a Server Ops bug to get IT to update the production copy on hg.mozilla.org. (CC :aravind)

Comment 7

[Arpad Borsos [:Swatinem]]

Comment on attachment 463302 [details] [diff] [review]
patch with files [pushed: comment 7]

Pushed as: http://hg.mozilla.org/hgcustom/pushlog/rev/eb0b62294161
Filed IT Bug as bug 585882.

Comment 8

[Arpad Borsos [:Swatinem]]

Attachment: tbpl patch

works now

Comment 9

[Markus Stange [:mstange]]

diff --git a/js/PushlogHTMLParser.js b/js/PushlogJSONParser.js

+      for(var i in data) {

Please leave a space between for/if/... and the opening bracket.

+          // I really don't want to mess around with moving tags

What are moving tags and why do they present a challenge?

+          var tags = $(tags).map(function(tag) {

I think this needs to be $(patch.tags).

+          // the json datasource returns the pushes in ASC order, the patches as well

Make this

          // Revert the order because we want most recent pushes / patches to
          // come first.

+          patches.unshift({rev: patch.node.substr(0,12), author: /([^<]+) /.exec(patch.author)[1],

Can you put the author extraction in its own line (and variable) and briefly explain the regex (e.g. "cut off before the email address", if that's correct)?

-    return Config.baseURL + "php/pipePushlog.php?url=" + repoName + "/pushloghtml%3Fstartdate=" + startDate + "%26enddate=" + endDate;
+    return "http://hg.mozilla.org/" + repoName + "/json-pushes?full=1&startdate=" + startDate + "&enddate=" + endDate;

Do you know why we were using %3F and %26 before?

Comment 10

[Arpad Borsos [:Swatinem]]

Attachment: tbpl patch

(In reply to comment #9)
> +          // I really don't want to mess around with moving tags
> What are moving tags and why do they present a challenge?

The tip tag is always on the most recent changeset. Its not a problem when we refresh the whole page but would be one when we only refresh the changing pushes.

> -    return Config.baseURL + "php/pipePushlog.php?url=" + repoName +
> "/pushloghtml%3Fstartdate=" + startDate + "%26enddate=" + endDate;
> +    return "http://hg.mozilla.org/" + repoName +
> "/json-pushes?full=1&startdate=" + startDate + "&enddate=" + endDate;
> 
> Do you know why we were using %3F and %26 before?

urlescape so the php script gets them as ? and &

Other nits fixed

Comment 11

[Markus Stange [:mstange]]

Comment on attachment 467367 [details] [diff] [review]
tbpl patch

Awesome.

Comment 12

[Arpad Borsos [:Swatinem]]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/50320b7c2e16
With another fix to escape html in push descriptions.

Comment 13

[Chris AtLee [:catlee]]

This includes branch information too!  \o/

I _think_ this lets us fix bug 466931

Comment 14

[Robert Helmer [:rhelmer]]

BTW it looks like this caused bug 588995 and possibly tickled a Minefield bug 589017 (the latter is not yet confirmed)

Comment 15

[Robert Helmer [:rhelmer]]

(In reply to comment #14)
> BTW it looks like this caused bug 588995 and possibly tickled a Minefield bug
> 589017 (the latter is not yet confirmed)

Bug 589017 is actually a TBPL bug (which appears to work in Chrome because of a suspected bug in Chrome's JSON parser).

Comment 16

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/73f8e97e96d9