[cr-audit-commits] Expose clients through separate parameter. |
||||
Issue descriptionPer code review feedback: "It seems unintuitive that this method gains access to the gerrit api via the AuditParams and their RepoCfg. "Params" and "Config" suggest that they should be static and just contain data, not access to external services. Maybe the RelevantCommit should contain a handle to the repo (and therefore gerrit instance) it came from?"
,
Sep 12 2017
,
Sep 12 2017
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/34bd5947cabedcb35a6939b8e8e5f9f582047911 commit 34bd5947cabedcb35a6939b8e8e5f9f582047911 Author: Roberto Carrillo <robertocn@google.com> Date: Wed Sep 13 00:10:46 2017 [cr-audit-commits] Remove clients from global scope. As suggested, a separate struct that contains the clients is passed to the rules and only lives for the life of the request. R=iannucci,vadimsh,stgao BUG= 763490 Change-Id: I8a232b3475d1b14b6550bd2730478efe4e8ddf92 Reviewed-on: https://chromium-review.googlesource.com/663772 Reviewed-by: Vadim Shtayura <vadimsh@chromium.org> Commit-Queue: Roberto Carrillo <robertocn@chromium.org> [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/commit_scanner.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/rules_config.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/commit_auditor.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/findit_rules_test.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/commit_scanner_test.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/findit_rules.go [modify] https://crrev.com/34bd5947cabedcb35a6939b8e8e5f9f582047911/go/src/infra/appengine/cr-audit-commits/app/utils.go
,
Sep 13 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by robert...@chromium.org
, Sep 8 2017My proposal is to add a new parameter to RuleFunc, a struct that contains clients to external services, initialized by the handler. This parameter will also be passed to the worker goroutines in commit_scanner. I am thinking of ``type ExternalClients struct{ milo miloClientInterface, gitiles gitilesClientInterface, monorail monorailClientInterface, ... }``