mergeRequests globally or per builder seems to not function

[bdbaddog]

I have an example where one builder fires off 20 triggers into another builder. The global c['mergeRequests'] is set to False (or a function which returns false) and it still merges the builds.

I'll attach example master.cfg

[bdbaddog]

master.cfg which demonstrates issue.

[bdbaddog]

Partial fix here:  https://github.com/bdbaddog/buildbot/commit/6ebcfa982c47706002151e2e29c25d9fffd815df

Still has a lot of my debugging code. This only addresses the mergeRequests arg to BuildConfig() and not the global.

[bdbaddog]

Sent pull request. Changes are all in:  https://github.com/bdbaddog/buildbot/commit/1e09bf84d254aff1adeef5915d28d4100ce520a9

[Dustin J. Mitchell]

remove unused BotMaster?.shouldMergeRequests

Refs #2008

Changeset: 29c9de940d34a0de8268f83a3b7b4f4229d25657

[Dustin J. Mitchell]

Builder configuration should default mergeRequests to None, not True

Without this change, mergeRequests is True regardless of the global setting, unless specifically set to something else in the BuilderConfig?.

This also fixes an error in _getMergeRequestFn, which was looking at self.master.mergeRequests, not self.botmaster.mergeRequests, and updates the tests to match and to have somewhat more readable inputs.

Fixes #2008.

Changeset: 984c004a6b04d616e727f6d984e179126fdc9cde

[Dustin J. Mitchell]

Allow cmergeRequests? = True

This is the default, and therefore unnecessary, but it shouldn't be explicitly disallowed. Refs #2008.

Changeset: 7928163ee56f657ea7bbd3791af5271ac04a87af

[Dustin J. Mitchell]

remove unused BotMaster?.shouldMergeRequests

Refs #2008

Changeset: f44c2e9fc13cda9e0e15f6bb4daf72cc7906dafd

[Dustin J. Mitchell]

Builder configuration should default mergeRequests to None, not True

Without this change, mergeRequests is True regardless of the global setting, unless specifically set to something else in the BuilderConfig?.

This also fixes an error in _getMergeRequestFn, which was looking at self.master.mergeRequests, not self.botmaster.mergeRequests, and updates the tests to match and to have somewhat more readable inputs.

Fixes #2008.

Changeset: 977eb41f40215a65215b6e74beac66a9f46b1801

[Dustin J. Mitchell]

Allow cmergeRequests? = True

This is the default, and therefore unnecessary, but it shouldn't be explicitly disallowed. Refs #2008.

Changeset: 88931660eb8d30d9f4515d6ae7b23486d9f9c15b

[dustin]

The actual fix here is in [984c004a6b04d616e727f6d984e179126fdc9cde], and is the one you had identified - an incorrect default value used when getting {{{mergeRequests}} from the builder's config dictionary. My patch reverts behavior to the previous behavior, rather than changing documentation and tests to match new (incorrect) behavior.