{"action":"created","issue":{"url":"https://api.github.com/repos/storj/storj/issues/3114","repository_url":"https://api.github.com/repos/storj/storj","labels_url":"https://api.github.com/repos/storj/storj/issues/3114/labels{/name}","comments_url":"https://api.github.com/repos/storj/storj/issues/3114/comments","events_url":"https://api.github.com/repos/storj/storj/issues/3114/events","html_url":"https://github.com/storj/storj/pull/3114","id":497843454,"node_id":"MDExOlB1bGxSZXF1ZXN0MzIwODk2NzMz","number":3114,"title":"scripts: add check-monitoring script to detect changes to monkit calls","user":{"login":"navillasa","id":26885324,"node_id":"MDQ6VXNlcjI2ODg1MzI0","avatar_url":"https://avatars2.githubusercontent.com/u/26885324?v=4","gravatar_id":"","url":"https://api.github.com/users/navillasa","html_url":"https://github.com/navillasa","followers_url":"https://api.github.com/users/navillasa/followers","following_url":"https://api.github.com/users/navillasa/following{/other_user}","gists_url":"https://api.github.com/users/navillasa/gists{/gist_id}","starred_url":"https://api.github.com/users/navillasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/navillasa/subscriptions","organizations_url":"https://api.github.com/users/navillasa/orgs","repos_url":"https://api.github.com/users/navillasa/repos","events_url":"https://api.github.com/users/navillasa/events{/privacy}","received_events_url":"https://api.github.com/users/navillasa/received_events","type":"User","site_admin":false},"labels":[{"id":1077048580,"node_id":"MDU6TGFiZWwxMDc3MDQ4NTgw","url":"https://api.github.com/repos/storj/storj/labels/Request%20Code%20Review","name":"Request Code Review","color":"80edbf","default":false},{"id":982933521,"node_id":"MDU6TGFiZWw5ODI5MzM1MjE=","url":"https://api.github.com/repos/storj/storj/labels/cla-signed","name":"cla-signed","color":"ededed","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2019-09-24T18:07:50Z","updated_at":"2019-10-14T20:34:19Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/storj/storj/pulls/3114","html_url":"https://github.com/storj/storj/pull/3114","diff_url":"https://github.com/storj/storj/pull/3114.diff","patch_url":"https://github.com/storj/storj/pull/3114.patch"},"body":"What: \r\n- Adds check-monitoring script to detect changes to monkit calls\r\n\r\nWhy:\r\n- The data science team relies on certain monkit metrics tied to function calls. They have queries that have dependencies on monkit metrics. This script allows us to generate a list of those metrics, and determine if changes affect those dependencies.\r\n\r\nTo test:\r\n- Remove one of the monkit calls that's followed by a `// locked` comment (or just remove the comment).\r\n- Run `make check-monitoring` - you should see a diff and error message because the lockfile was not updated.\r\n\r\nPlease describe the tests:\r\n - Test 1:\r\n - Test 2:\r\n \r\nPlease describe the performance impact: This adds one more check during the \"lint\" jenkins stage. This check walks the entire package/file heirarchy and iterates over the ast of each file multiple times.\r\n\r\n## Code Review Checklist (to be filled out by reviewer)\r\n - [ ] Does the PR describe what changes are being made?\r\n - [ ] Does the PR describe why the changes are being made?\r\n - [ ] Does the code follow [our style guide](https://github.com/storj/docs/blob/master/code/Style.md)?\r\n - [ ] Does the code follow [our testing guide](https://github.com/storj/docs/blob/master/code/Testing.md)?\r\n - [ ] Is the PR appropriately sized? (If it could be broken into smaller PRs it should be)\r\n - [ ] Does the new code have enough tests? (*every* PR should have tests or justification otherwise. Bug-fix PRs especially)\r\n - [ ] Does the new code have enough documentation that answers \"how do I use it?\" and \"what does it do?\"? (both source documentation and [higher level](https://github.com/storj/docs), diagrams?)\r\n - [ ] Does any documentation need updating?\r\n - [ ] Do the database access patterns make sense?\r\n"},"comment":{"url":"https://api.github.com/repos/storj/storj/issues/comments/541905676","html_url":"https://github.com/storj/storj/pull/3114#issuecomment-541905676","issue_url":"https://api.github.com/repos/storj/storj/issues/3114","id":541905676,"node_id":"MDEyOklzc3VlQ29tbWVudDU0MTkwNTY3Ng==","user":{"login":"navillasa","id":26885324,"node_id":"MDQ6VXNlcjI2ODg1MzI0","avatar_url":"https://avatars2.githubusercontent.com/u/26885324?v=4","gravatar_id":"","url":"https://api.github.com/users/navillasa","html_url":"https://github.com/navillasa","followers_url":"https://api.github.com/users/navillasa/followers","following_url":"https://api.github.com/users/navillasa/following{/other_user}","gists_url":"https://api.github.com/users/navillasa/gists{/gist_id}","starred_url":"https://api.github.com/users/navillasa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/navillasa/subscriptions","organizations_url":"https://api.github.com/users/navillasa/orgs","repos_url":"https://api.github.com/users/navillasa/repos","events_url":"https://api.github.com/users/navillasa/events{/privacy}","received_events_url":"https://api.github.com/users/navillasa/received_events","type":"User","site_admin":false},"created_at":"2019-10-14T20:34:19Z","updated_at":"2019-10-14T20:34:19Z","author_association":"CONTRIBUTOR","body":"> A couple of comments and I also want to raise the following point.\r\n> \r\n> I've realized that currently, the tool is obeying to `// locked` (with the implied white space between `//` and `locked`). Most of the tools, if not all, that I used which use comments for annotating code have the instructions without white space between the `//` and the `<command>`. I was thinking that we could follow that convention.\r\n> \r\n> I've found that [Golangci-lint has an entry in its README which mentions that](https://github.com/golangci/golangci-lint#nolint).\r\n> \r\n> Nonetheless, this change could be left for a future improvement registered in a ticket if we need to merge this PR ASAP.\r\n\r\nThank you for the feedback and review @ifraixedes ! I think that's a good idea to follow the golangci-lint convention, so I've gone ahead and changed occurrences from `// locked` to `//locked`."}}
{"actor":{"display_login":"navillasa"}}