Closed erwinmombay closed 4 years ago
I think having a self-contained app that only handles this is a better choice. There's no reason to tack on a feature into the owners bot, and with Probot and the lessons learned from previous apps that we wrote it should be super simple to create this app
Agreed with @danielrozenberg , see https://github.com/ampproject/amp-github-apps/issues/283#issuecomment-521016027
@rcebulko @danielrozenberg I suggested handling this via the owners bot for a couple reasons:
What I had in mind was for the owners bot to special-case renovate PRs and examine the dependency being upgraded. After one or more owners are identified, the bot can go on to use the same logic as it would for any other PR, and watch for review events to determine if the owners check is satisfied.
/cc @erwinmombay who had input on this
I recently found the depcheck package, which could be extremely useful here. It identifies which files depend on each package listed as a dependence or devDependency is package.json (including transitive dependencies). I've included the output of running it on the ampproject/amphtml
root directory below. I'd propose that the renovate-bot assignment logic just identify the dependency being upgraded and assign reviewers using the same algorithm, as if the PR were modifying all of those files. That would keep it simple, consistent, and easy to debug the owners set (as opposed to looking at line numbers, commit histories, etc.).
{
"dependencies":[
"moment",
"preact",
"preact-compat",
"prop-types",
"web-activities"
],
"devDependencies":[
"@babel/plugin-proposal-class-properties",
"@babel/preset-env",
"babel-plugin-istanbul",
"codecov",
"derequire",
"doctrine",
"envify",
"eslint",
"eslint-config-prettier",
"eslint-plugin-eslint-plugin",
"eslint-plugin-jasmine",
"jest-cli",
"jest-dot-reporter",
"jest-silent-reporter",
"json5",
"karma-browserify",
"karma-chai",
"karma-chrome-launcher",
"karma-coverage-istanbul-reporter",
"karma-edge-launcher",
"karma-firefox-launcher",
"karma-fixture",
"karma-html2js-preprocessor",
"karma-ie-launcher",
"karma-mocha",
"karma-mocha-reporter",
"karma-safari-launcher",
"karma-sauce-launcher",
"karma-simple-reporter",
"karma-sinon-chai",
"karma-source-map-support",
"karma-super-dots-reporter",
"npm-exact-versions",
"path",
"prettier",
"react",
"react-addons-shallow-compare",
"react-dom",
"react-externs",
"react-with-direction",
"sinon-chai",
"traverse",
"try-net-connect",
"uglifyify"
],
"missing":{
"ava":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/csvify-size/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/test.js"
]
},
"using":{
"babel-eslint":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-amphtml-internal":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-chai-expect":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-google-camelcase":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-jsdoc":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-notice":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-prettier":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-sort-imports-es6-autofix":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"eslint-plugin-sort-requires":[
"/usr/local/google/home/rcebulko/amphtml/.eslintrc"
],
"minimist":[
"/usr/local/google/home/rcebulko/amphtml/babel.config.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/amp4test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/build.conf.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/internal-version.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/a4a.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-generator/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/firebase.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/integration.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/pr-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-3p-github-pr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-github-issues.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/report-test-status.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/runtime-test-base.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/serve.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/unit.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/update-packages.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/validator.js"
],
"express":[
"/usr/local/google/home/rcebulko/amphtml/build-system/amp4test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/recaptcha-router.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/routes/analytics.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/routes/list.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/routes/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/routes/user-location.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/test-server.js"
],
"multer":[
"/usr/local/google/home/rcebulko/amphtml/build-system/amp4test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/recaptcha-router.js"
],
"fuse.js":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/api/api.js"
],
"bluebird":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-self.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/util/listing.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-video-testbench.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/recaptcha-router.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/csvify-size/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-3p-github-pr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-github-issues.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js"
],
"amphtml-validator":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-amphtml-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-self.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-template.js"
],
"chai":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-amphtml-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-html.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-file-list.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-self.js"
],
"jsdom":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/test-amphtml-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-index/test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app-video-testbench.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js"
],
"baconipsum":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js"
],
"body-parser":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/test-server.js"
],
"formidable":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js"
],
"request":[
"/usr/local/google/home/rcebulko/amphtml/build-system/app.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-3p-github-pr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-github-issues.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js"
],
"@babel/helper-plugin-test-runner":[
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-amp-mode-transformer/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-is_dev-constant-transformer/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-is_minified-constant-transformer/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-html-template/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-amp-extension-call/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-prune-namespace/test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-version-call/test/index.js"
],
"html-minifier":[
"/usr/local/google/home/rcebulko/amphtml/build-system/babel-plugins/babel-plugin-transform-html-template/index.js"
],
"babel-plugin-minify-replace":[
"/usr/local/google/home/rcebulko/amphtml/build-system/build.conf.js"
],
"babel-plugin-filter-imports":[
"/usr/local/google/home/rcebulko/amphtml/build-system/build.conf.js"
],
"google-closure-compiler":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/closure-compile.js"
],
"ansi-colors":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/closure-compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/ctrlcHandler.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/build-targets.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/build.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/dist-bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/local-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/single-pass-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/e2e-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/remote-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/validator-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/utils.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/yarn-checks.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/visual-diff-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-exact-versions.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/codecov-upload.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/csvify-size/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-generator/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/firebase.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/json-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/nailgun.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/presubmit-checks.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-3p-github-pr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-github-issues.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/report-test-status.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers-unit.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/runtime-test-base.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/serve.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/update-packages.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/travis.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/typescript.js",
"/usr/local/google/home/rcebulko/amphtml/bundles.config.js"
],
"cli-highlight":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/closure-compile.js"
],
"del":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/clean.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/generate-vendor-jsons.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"fs-extra":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/codecov-upload.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/create-golden-css/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/compile-expr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-generator/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/firebase.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/update-packages.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/typescript.js"
],
"gulp":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/ava.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/babel-plugin-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/build.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/create-module-compatible-es5-bundle.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/generate-vendor-jsons.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/json-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/presubmit-checks.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js",
"/usr/local/google/home/rcebulko/amphtml/gulpfile.js"
],
"gulp-if":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js"
],
"gulp-nop":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js"
],
"gulp-rename":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js"
],
"gulp-sourcemaps":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/compile.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/create-module-compatible-es5-bundle.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"regexp.escape":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/shorten-license.js"
],
"pumpify":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/shorten-license.js"
],
"gulp-regexp-sourcemaps":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/shorten-license.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/create-module-compatible-es5-bundle.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"@babel/core":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js"
],
"babelify":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/runtime-test-base.js"
],
"browserify":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"dev-null":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"fancy-log":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/ctrlcHandler.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-exact-versions.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-types.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/codecov-upload.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/csvify-size/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-generator/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/firebase.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/json-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/nailgun.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/presubmit-checks.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-3p-github-pr.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/process-github-issues.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/report-test-status.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers-unit.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/runtime-test-base.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/serve.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/update-packages.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/travis.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/typescript.js",
"/usr/local/google/home/rcebulko/amphtml/bundles.config.js"
],
"magic-string":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"@jridgewell/resorcery":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"tempy":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/sources.js"
],
"terser":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"through2":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/json-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/presubmit-checks.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/todos.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"topological-sort":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"babel-plugin-transform-es2015-modules-commonjs":[
"/usr/local/google/home/rcebulko/amphtml/build-system/compile/single-pass.js"
],
"minimatch":[
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/build-targets.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers-unit.js"
],
"request-promise":[
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/utils.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/report-test-status.js"
],
"atob":[
"/usr/local/google/home/rcebulko/amphtml/build-system/pr-check/visual-diff-tests.js"
],
"ast-replace":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"acorn-globals":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"escodegen":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"rocambole":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"event-stream":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"commander":[
"/usr/local/google/home/rcebulko/amphtml/build-system/scope-require.js"
],
"is-running":[
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js"
],
"morgan":[
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js"
],
"gulp-webserver":[
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js"
],
"connect-header":[
"/usr/local/google/home/rcebulko/amphtml/build-system/server.js"
],
"gulp-ava":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/ava.js"
],
"gulp-jest":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/babel-plugin-tests.js"
],
"@octokit/rest":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/bundle-size.js"
],
"gulp-git":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/changelog.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/release-tagging.js"
],
"markdown-link-check":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/check-links.js"
],
"jison":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/compile-expr.js"
],
"gulp-file":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/generate-vendor-jsons.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"gulp-watch":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/css.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/extension-helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js"
],
"ava":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/csvify-size/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/test.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/prepend-global/test.js"
],
"vinyl-source-stream":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dep-check.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"globs-to-files":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dev-dashboard-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dist.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers-unit.js"
],
"mocha":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/dev-dashboard-tests.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/mocha-ci-reporter.js"
],
"plugin-error":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"postcss":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js"
],
"text-table":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/get-zindex/index.js",
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"vinyl-buffer":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"watchify":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/helpers.js"
],
"autoprefixer":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js"
],
"cssnano":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js"
],
"postcss-import":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/jsify-css.js"
],
"gulp-eslint":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js"
],
"gulp-eslint-if-fixed":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js"
],
"lazypipe":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/lint.js"
],
"sleep-promise":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/nailgun.js"
],
"require-hijack":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/nailgun.js"
],
"opn":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js"
],
"karma":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers.js"
],
"find-imports":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/runtime-test/helpers-unit.js"
],
"nodemon":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/serve.js"
],
"gzip-size":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"pretty-bytes":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/size.js"
],
"gulp-jsonlint":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js"
],
"gulp-jsonminify":[
"/usr/local/google/home/rcebulko/amphtml/build-system/tasks/vendor-configs.js"
],
"typescript":[
"/usr/local/google/home/rcebulko/amphtml/build-system/typescript.js"
],
"tsickle":[
"/usr/local/google/home/rcebulko/amphtml/build-system/typescript.js"
],
"gulp-help":[
"/usr/local/google/home/rcebulko/amphtml/gulpfile.js"
],
"lolex":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-access/0.1/test/test-amp-access-iframe.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-access/0.1/test/test-amp-access-client.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-access/0.1/test/test-amp-access-server-jwt.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-access/0.1/test/test-amp-access-server.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-ad/0.1/test/test-concurrent-load.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-analytics/0.1/test/test-cookie-writer.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-analytics/0.1/test/test-events.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-analytics/0.1/test/test-requests.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-analytics/0.1/test/test-transport.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-bind/0.1/test/integration/test-bind-impl.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-consent/0.1/test/test-consent-policy-manager.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-display/0.1/test/test-amp-date-display.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/integration/test-integration-amp-date-picker-actions.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/integration/test-integration-maximum-nights.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/integration/test-integration-dates-attributes.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/test-amp-date-picker-state.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/test-amp-date-picker-bind.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/test-amp-date-picker.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/test/test-wrapper-maximum-nights.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-image-lightbox/0.1/test/test-amp-image-lightbox.js",
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-sidebar/0.1/test/test-amp-sidebar.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-3p-environment.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-ad-cid.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-cache-cid-api.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-cid.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-cookies.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-custom-element.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-jank-meter.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-layout-delay-meter.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-performance.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-position-observer.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-preconnect.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-render-delaying-services.js",
"/usr/local/google/home/rcebulko/amphtml/test/unit/test-yield.js"
],
"web-animations-js":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-animation/0.1/web-animations-polyfill.js"
],
"rrule":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-date-picker/0.1/dates-list.js"
],
"@ampproject/animations":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-lightbox-gallery/0.1/amp-lightbox-gallery.js"
],
"set-dom":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-list/0.1/amp-list.js"
],
"@ampproject/worker-dom":[
"/usr/local/google/home/rcebulko/amphtml/extensions/amp-script/0.1/amp-script.js"
],
"promise-pjs":[
"/usr/local/google/home/rcebulko/amphtml/src/polyfills/promise.js"
],
"document-register-element":[
"/usr/local/google/home/rcebulko/amphtml/src/polyfills.js",
"/usr/local/google/home/rcebulko/amphtml/src/service/extensions-impl.js"
],
"dompurify":[
"/usr/local/google/home/rcebulko/amphtml/src/purifier.js"
],
"json-stable-stringify":[
"/usr/local/google/home/rcebulko/amphtml/test/_init_tests.js"
],
"chai-as-promised":[
"/usr/local/google/home/rcebulko/amphtml/test/chai-as-promised/chai-as-promised.js"
],
"sinon":[
"/usr/local/google/home/rcebulko/amphtml/test/e2e/test-controller-promise.js"
],
"fetch-mock":[
"/usr/local/google/home/rcebulko/amphtml/testing/describes.js"
],
"react-dates":[
"/usr/local/google/home/rcebulko/amphtml/third_party/react-dates/index.js"
]
},
"invalidFiles":{
},
"invalidDirs":{
}
}
Using this tool sounds like a good idea in general.
There's one corner case: A dependency used in several places will require reviews from several owners, which seems unnecessary for most tooling dependencies, for example, the one used to parse argv
.
If this turns out to be a problem in practice, maybe you can set a threshold?
That makes sense, though in theory all those users are stakeholders to some extent. I could see an approach that varies depending on the upgrade:
Patch: Any stakeholder Minor Upgrade: Max 2~3 Major Upgrade: Full owners coverage
Ex. that major version upgrade to probot (in the amp-github-apps
repo) would have been fine for some of the apps, but would have completely broken others. Major version upgrades to dependencies should be rare enough that it wouldn't be a significant inconvenience.
Thoughts?
Yep, this plan SGTM. (Should be easy to fine tune if this does become a problem.)
Bumping this to a P2 until the above mentioned issue/process is addressed, so we avoid over-engineering this.
For a long-term solution, we should figure out our overall policy toward dependencies and how they should be regarded w.r.t. security. To address the issue of renovate-spam + review load on the build cop, I propose the following:
major
and minor
updatesThese tend to introduce more risk and breakages, and should be tested and on an individual basis. They are also much rarer; based on looking at the last couple dozen renovate PRs, less than one third are major or minor updates. Finally, since important security fixes are likely to be present in minor updates, these should be reviewed sooner and more frequently.
renovate
to group all patch updates together and create a PR at most every N (2?) weeksThese changes are generally very small (minor bugfixes) and have no discernible effect. . Despite this, they account for around two-thirds of all the renovate PRs sitting in a Build Cop's list, and each one is subject to the risk of flaky tests blocking it from easy submission. These patches could be safely grouped together, and updates could be made fairly infrequently without any real downsides.
Nearly 90% of all renovate PRs are eventually approved and merged by someone from the infrastructure team anyway. As a build-cop responsibility, monitoring and merging renovate PRs is a tenuously-related responsibility at best, and a fairly challenging task for some. Within the infra team, we have the context of seeing the effects of many renovate PRs over time, and have a pretty good sense of what is a flake versus a real test failure introduced by a changed dependency (particularly since many of these dependencies were introduced and maintained by wg-infra anyway)
By assigning wg-infra directly as a reviewer, we could spread the load over the team, rather than assigning the renovate PRs semi-arbitrarily and eventually taking ownership of them later on. Doing this would also remove that responsibility from the build cop, allowing them to better focus on the core job of build cop: keeping master green.
@rsimha WDYT?
@rsimha Bump
@rcebulko Seems like I was inadvertently unsubscribed from notifications for this issue. Apologies for missing your last two @mentions.
- Keep the current system in place for
major
andminor
updates
Agree. Major and minor updates should be treated like normal PRs, and should be reviewed and merged individually. (There are some packages like closure compiler that use version strings like v20200315
, and I just verified that they are treated as major updates, so this approach will work.)
- Instruct
renovate
to group all patch updates together and create a PR at most every N (2?) weeks
Aha, I wasn't aware that patch updates could be grouped by renovate. I like the idea of doing so, but I don't think the time between updates should be as large as 2 weeks. We'd have tens of updates per PR, making it hard to pinpoint the cause of flakes / failures. How about we start with one PR per day for patch updates, and increase the number as we go along to see where the sweet spot lies? WDYT?
Edit: Relevant Renovate FR for grouping of updates: https://github.com/renovatebot/renovate/issues/1342
- Assign all renovate PRs to @ampproject/wg-infra
The original intent behind this issue was to spread the load across the larger team, and assign PRs to the working group who "owns" a given dependency, since they're in the best position to determine the safety of an upgrade. (e.g. build-system/
-> @wg-infra
, src/
-> @wg-runtime
, ads/
-> @wg-ads
, etc.) I realize this will require us to inspect where a dev-dependency is being used and make a best guess if it's used in multiple places.
Let's chat more offline and figure out a plan. Thanks for the discussion thus far!
This issue may be largely mediated by the new grouping of package updates (see master renovate issue) by the location of the package.json
file. Let's monitor renovate PRs this upcoming week (once they are enabled) and, if the PR load is reasonable, we may be able to close this without introducing the complexity of file-line-level rules.
Bumping this thread. According to the master issue, it looks like the PR volume has gone way down with the introduction of update grouping. If this is sufficient, I think we can close this issue. If not, we could add a top-level OWNERS rule for **/package.json
to enable notification comments tagging @ampproject/build-cop
, so the build cop gets CC'd on package update PRs created during their rotation.
@rsimha WDYT about this?
Agree that the work you did in reducing PR volume is sufficient. Thanks for all the work there!
/per @rsimha Incoming renovate PRs tend to get ignored by most build cops. The owners bot is in a good position to assign each incoming PR to the current build cop for prompt attention.
We can do two things when a renovate PR is initially created, or a new commit is pushed:
/cc @danielrozenberg who is working on an API to return the github ID of the current build cop /cc @mrjoro