Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Description • 13 years ago

Right now people still get error emails when they have requested -n

	Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Comment 1 • 13 years ago

Need this for at least Try but as discussed in IRC it could be useful for other sendchanges too.  The try_mailer determines if it will send an email based on try syntax which it gets from build.source.changes[-1].comments (not from pushlog like the try_parser) so for test/talos builds this means it gets blank comments and sends the default emails which is any non-successful result.
 
passed test-masters.sh and local testing of sendchanges from a build:

    master: dev-master01.build.scl1.mozilla.com:9009
    branch: try-macosx64-talos
    revision: a81418f9ece4
    comments: adding 2 try: -b o -p macosx64 -n
    user: lsblakk@mozilla.com
    files: ['http://example.com']
    properties: [('buildid', '20110831103935'), ('builduid', '253572a5563e4c4cbf717652ada266a9')]

	bhearsum@mozilla.com (:bhearsum)
	Comment 2 • 13 years ago

Comment on attachment 557367 [details] [diff] [review]
forward the build comments to test/talos sendchanges

Review of attachment 557367 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/factory.py
@@ +5424,5 @@
>                   branch=self.unittestBranch,
>                   files=[WithProperties('%(packageUrl)s'),
>                          WithProperties('%(testsUrl)s')],
>                   user="sendchange-unittest",
> +                 comments=WithProperties('%(comments)s'),

I don't think this will work here, because it inherits MozillaBuildFactory instead of MercurialBuildFactory. Same for the Try and CC variants. r=me with these hunks dropped.

	Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Comment 3 • 13 years ago

Comment on attachment 557367 [details] [diff] [review]
forward the build comments to test/talos sendchanges

http://hg.mozilla.org/build/buildbotcustom/rev/4d9e3e2224d2

took out the UnittestBuildFactory, TryUnittestBuildFactory, and CCUnittestBuildFactory lines so that this is only on factories that inherit from MercurialBuildFactory.

	Aki Sasaki (not active)
	Comment 4 • 13 years ago

Comment on attachment 557367 [details] [diff] [review]
forward the build comments to test/talos sendchanges

Backed out due to bug 684086.
Noticed that the 'comments' field will only be set if self.graphServer; sendchanges will raise an exception otherwise.

	Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Comment 5 • 13 years ago

caused bug 684086 because addBuildInfoSteps() is only called if self.graphServer
need to rework this.

	Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Comment 6 • 13 years ago

Tested this with try as well as m-c builders on my local machine.  This version addresses the comments being set no matter what as I moved where the property gets set to a more global step, protected the lambda return with an empty string, and also left in the comments=WithProperties('%(comments:-)s') for extra sureness.  This allows builds to be forced - comments end up as "" and for builds triggered by source changes to have the comments set to the push comments and passed along to test/talos sendchanges.

	Aki Sasaki (not active)
	Comment 7 • 13 years ago

Comment on attachment 559240 [details] [diff] [review]
(v.2) forward the build comments to test/talos sendchanges

Very thorough, thank you.
I was wondering what happens if 'who' ends up being "" or None, which might bork SendChangeStep. But if anything, that would have already broken things, and this patch would possibly improve that.

	Lukas Blakk [:lsblakk] use ?needinfo Assignee
	Comment 8 • 13 years ago

Comment on attachment 559240 [details] [diff] [review]
(v.2) forward the build comments to test/talos sendchanges

landed on default http://hg.mozilla.org/build/buildbotcustom/rev/6cbf85b01236

	Armen [:armenzg]
	Comment 9 • 13 years ago

Merged to production and reconfigured.

	Mozilla RelEng Bot
	Comment 10 • 13 years ago

Try run for a5b0b5b04df1 is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=a5b0b5b04df1
Results (out of 7 total builds):
    success: 7
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/lsblakk@mozilla.com-a5b0b5b04df1