)]}'
{"zuul/configloader.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"a04914a7901d6b96eb30798fa7f5b3fd314818c3","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"        if conf.get(\u0027admin-rules\u0027) is not None:"},{"line_number":1473,"context_line":"            tenant.authorization_rules \u003d conf[\u0027admin-rules\u0027]"},{"line_number":1474,"context_line":"        if conf.get(\u0027report-build-page\u0027) is not None:"},{"line_number":1475,"context_line":"            tenant.report_build_page \u003d conf[\u0027report-build-page\u0027]"},{"line_number":1476,"context_line":"        tenant.web_root \u003d conf.get(\u0027web-root\u0027, self.scheduler.web_root)"},{"line_number":1477,"context_line":"        tenant.allowed_triggers \u003d conf.get(\u0027allowed-triggers\u0027)"},{"line_number":1478,"context_line":"        tenant.allowed_reporters \u003d conf.get(\u0027allowed-reporters\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_5e17d946","line":1475,"updated":"2019-08-08 17:08:11.000000000","message":"Minor simplification potential:\n\n  tenant.report_build_page \u003d conf.get(\u0027report-build-page\u0027, False)\n\nThen you can avoid the if. However the if check is what is used for everything here so probably better to rewrite the whole lot if we decide that reads better.","commit_id":"f13cc924df19d12b3ca76dd51dce82e256d52d9a"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"ee08d81c2af43cdd0c1c723f59bbe711bf562cd0","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"        if conf.get(\u0027admin-rules\u0027) is not None:"},{"line_number":1473,"context_line":"            tenant.authorization_rules \u003d conf[\u0027admin-rules\u0027]"},{"line_number":1474,"context_line":"        if conf.get(\u0027report-build-page\u0027) is not None:"},{"line_number":1475,"context_line":"            tenant.report_build_page \u003d conf[\u0027report-build-page\u0027]"},{"line_number":1476,"context_line":"        tenant.web_root \u003d conf.get(\u0027web-root\u0027, self.scheduler.web_root)"},{"line_number":1477,"context_line":"        tenant.allowed_triggers \u003d conf.get(\u0027allowed-triggers\u0027)"},{"line_number":1478,"context_line":"        tenant.allowed_reporters \u003d conf.get(\u0027allowed-reporters\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9e6eb194","line":1475,"updated":"2019-08-08 17:15:17.000000000","message":"That does make it slightly less future-proof, since when the implicit default is later changed in the Tenant class this would need updating too. As it is now, only the initial state of the classvar needs to be changed to change the default (which I gather is a planned future state).\n\nAn alternative hybrid with your suggestion but maintaining the future flexibility characteristic might be:\n\n    tenant.report_build_page \u003d conf.get(\u0027report-build-page\u0027, tenant.report_build_page)","commit_id":"f13cc924df19d12b3ca76dd51dce82e256d52d9a"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"c9fc3c5ead181624620157e3b7a531accc420ca7","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"        if conf.get(\u0027admin-rules\u0027) is not None:"},{"line_number":1473,"context_line":"            tenant.authorization_rules \u003d conf[\u0027admin-rules\u0027]"},{"line_number":1474,"context_line":"        if conf.get(\u0027report-build-page\u0027) is not None:"},{"line_number":1475,"context_line":"            tenant.report_build_page \u003d conf[\u0027report-build-page\u0027]"},{"line_number":1476,"context_line":"        tenant.web_root \u003d conf.get(\u0027web-root\u0027, self.scheduler.web_root)"},{"line_number":1477,"context_line":"        tenant.allowed_triggers \u003d conf.get(\u0027allowed-triggers\u0027)"},{"line_number":1478,"context_line":"        tenant.allowed_reporters \u003d conf.get(\u0027allowed-reporters\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_9e50f160","line":1475,"updated":"2019-08-08 17:29:09.000000000","message":"Yeah, I wrote it that way originally, but matched the rest to keep more of the defaults for this in the model.  But maybe we should move them all in here, where other defaults are.\nI like Jeremy\u0027s idea too.","commit_id":"f13cc924df19d12b3ca76dd51dce82e256d52d9a"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"7d8f0ae0d98d3e5f0d2cfc7a81e3ef1cde69d059","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"        if conf.get(\u0027admin-rules\u0027) is not None:"},{"line_number":1473,"context_line":"            tenant.authorization_rules \u003d conf[\u0027admin-rules\u0027]"},{"line_number":1474,"context_line":"        if conf.get(\u0027report-build-page\u0027) is not None:"},{"line_number":1475,"context_line":"            tenant.report_build_page \u003d conf[\u0027report-build-page\u0027]"},{"line_number":1476,"context_line":"        tenant.web_root \u003d conf.get(\u0027web-root\u0027, self.scheduler.web_root)"},{"line_number":1477,"context_line":"        tenant.allowed_triggers \u003d conf.get(\u0027allowed-triggers\u0027)"},{"line_number":1478,"context_line":"        tenant.allowed_reporters \u003d conf.get(\u0027allowed-reporters\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_c4a8d68d","line":1475,"in_reply_to":"7faddb67_9e50f160","updated":"2019-08-08 19:07:46.000000000","message":"I also like Jeremy\u0027s idea as a possible future refactoring.","commit_id":"f13cc924df19d12b3ca76dd51dce82e256d52d9a"}]}