Deduplicate config, auto-remove #g-ceo and #handbook labels when appropriate, improve comments (#12915)

- Deduplicate config for DRI vs CODEOWNERS (this eliminates extra
notifications folks were receiving)
- auto-remove #g-ceo and #handbook labels when appropriate
- improve comments
This commit is contained in:
Mike McNeil 2023-07-22 14:07:54 -05:00 committed by GitHub
parent 199dbf9211
commit 6460893a5c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 94 additions and 61 deletions

View file

@ -10,10 +10,16 @@
#
# > How? This "requiredness" is provided natively by GitHub. If a team is specified, then
# > the logic behaves slightly differently. See GitHub's latest documentation on CODEOWNERS
# > for more information.
# > for more information. CODEOWNERS is especially useful for paths that usually end up
# > in PRs with lots of other reviewers.)
#
# ⚠️ For file paths not listed, the DRI is indicated elsewhere (website/config/custom.js).
# (In either case, the DRI is automatically requested for review when changes are proposed.)
# ⚠️ For file paths not listed, the DRI is instead indicated in website/config/custom.js.
# Regardless of whether a path's DRI is configured in CODEOWNERS or custom.js, the DRI is
# automatically requested for review when changes are proposed.
# [!] But beware: No path should ever be configured as a DRI in both CODEOWNERS _and_
# the website config.
# [!] In addition, no path should ever be configured in CODEOWNERS if there is ALSO one
# of its ancestral paths configured in website/config/custom.js.
#
# ✅ Some paths also have multiple individuals who are allowed to make changes without review,
# even though they are not the DRI. These are called "maintainers".
@ -25,7 +31,7 @@
##############################################################################################
# Golang files and other files related to the core product backend.
# 🚀 Golang files and other files related to the core product backend.
# (1 or more Golang-literate engineers is required to review changes.)
# FUTURE: Look for a way to not have this notify every single person in this "github team".
##############################################################################################
@ -36,14 +42,14 @@ go.mod @fleetdm/go
/cmd/ @fleetdm/go
##############################################################################################
# React files and other files related to the core product frontend.
# 🚀 React files and other files related to the core product frontend.
# (1 or more React-literate engineers is required to review changes.)
# FUTURE: Look for a way to not have this notify every single person in this "github team".
##############################################################################################
/frontend/ @fleetdm/frontend
##############################################################################################
# Config as code for infrastructure, internal security and IT use cases, and more.
# 🚀 Config as code for infrastructure, internal security and IT use cases, and more.
# (1 or more infra-literate engineers is required to review changes.)
# FUTURE: Look for a way to not have this notify every single person in this "github team".
##############################################################################################
@ -52,17 +58,33 @@ go.mod @fleetdm/go
/terraform/ @fleetdm/infra
##############################################################################################
# Key handbook pages w/ required reviewers
# ⚗️ Reference, config surface, built-in queries, API, and other documentation.
#
# (Especially useful for paths that tend to end up in PRs with lots of other reviewers)
# (see website/config/custom.js for DRIs of other paths not listed here)
##############################################################################################
/handbook/company/development-groups.md @mikermcneil
/handbook/company/why-this-way.md @mikermcneil
/handbook/company/README.md @mikermcneil
/handbook/business-operations/README.md @mikermcneil
/docs/ @rachaelshaw
/schema/ @rachaelshaw #« Data tables (osquery/fleetd schema) documentation
CHANGELOG.md @lukeheath
/docs @rachaelshaw
/docs/Using-Fleet/REST-API.md @rachaelshaw # « REST API reference documentation
/docs/Contributing/API-for-contributors.md @rachaelshaw # « Advanced / contributors-only API reference documentation
/schema @rachaelshaw # « Data tables (osquery/fleetd schema) documentation
##############################################################################################
# 🫧 Pricing and features
#
# (see website/config/custom.js for DRIs of other paths not listed here)
##############################################################################################
/website/views/pages/pricing.ejs @mikermcneil # « CEO is DRI for pricing
/handbook/product/pricing-features-table.yml @mikermcneil # « CEO is current DRI for product marketing (e.g. features table)
##############################################################################################
# 🦿 Handbook
#
# (see website/config/custom.js for DRIs of other paths not listed here)
##############################################################################################
/handbook/company/development-groups.md @mikermcneil
/handbook/company/why-this-way.md @mikermcneil
/handbook/company/README.md @mikermcneil
/handbook/README.md @mikermcneil # « This is the "Table of contents" landing page

View file

@ -107,7 +107,8 @@ module.exports = {
let issueOrPr = (pr || issue || undefined);
let ghNoun = this.req.get('X-GitHub-Event');// See https://developer.github.com/v3/activity/events/types/
sails.log('Received GitHub webhook request:', ghNoun, action, {sender, repository, comment, label, issueOrPr});
sails.log(`Received GitHub webhook request: ${ghNoun} :: ${action}: ${require('util').inspect({sender, repository: _.isObject(repository) ? repository.full_name : undefined, comment, label, issueOrPr}, {depth:null})}`);
if (
(ghNoun === 'issues' && ['opened','reopened'].includes(action))
) {
@ -306,8 +307,23 @@ module.exports = {
DRI_BY_PATH = sails.config.custom.githubRepoDRIByPath;
} else {
// Other repos don't have this configured.
// FUTURE: Configure it for them
}
let existingLabels = _.isArray(issueOrPr.labels) ? _.pluck(issueOrPr.labels, 'name') : [];
// Look up already-requested reviewers
// (for use later in minimizing extra notifications for editing PRs to contain new changes
// while also still doing appropriate review requests. Also for determining whether
// to apply the #g-ceo label)
//
// The "requested_reviewers" key in the pull request object:
// - https://developer.github.com/v3/activity/events/types
// - https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads?actionType=edited#pull_request
// - https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request
let alreadyRequestedReviewers = _.isArray(issueOrPr.requested_reviewers) ? _.pluck(issueOrPr.requested_reviewers, 'login') : [];
alreadyRequestedReviewers.map((username) => username.toLowerCase());// « make sure they are all lowercased
// Request review from DRI
// History: https://github.com/fleetdm/fleet/pull/12786) (only relevant for paths NOT in the CODEOWNERS file)
// (Draft PRs are skipped)
@ -315,13 +331,6 @@ module.exports = {
let reviewers = [];//« GitHub usernames of people to request review from.
// Look up already-requested reviewers
// (for use in minimizing extra notifications for editing PRs to contain new changes
// while also still doing appropriate review requests)
// [?] https://developer.github.com/v3/activity/events/types
// [?] The "requested_reviewers" key in the pull request object: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request
let alreadyRequestedReviewers = _.pluck(issueOrPr.requested_reviewers, 'login');
// Look up paths
// [?] https://docs.github.com/en/rest/reference/pulls#list-pull-requests-files
let changedPaths = _.pluck(await sails.helpers.http.get(`https://api.github.com/repos/${owner}/${repo}/pulls/${prNumber}/files`, {
@ -389,19 +398,6 @@ module.exports = {
}//fi
// Check whether auto-approval is warranted.
let isAutoApproved = await sails.helpers.githubAutomations.getIsPrPreapproved.with({
repo: repo,
prNumber: prNumber,
githubUserToCheck: sender.login,
isGithubUserMaintainerOrDoesntMatter: GITHUB_USERNAMES_OF_BOTS_AND_MAINTAINERS.includes(sender.login.toLowerCase())
});
let isHandbookPR = false;
if(repo === 'fleet'){
isHandbookPR = await sails.helpers.githubAutomations.getIsPrOnlyHandbookChanges.with({prNumber: prNumber});
}
// Check whether the "main" branch is currently frozen (i.e. a feature freeze)
// [?] https://docs.mergefreeze.com/web-api#get-freeze-status
let mergeFreezeMainBranchStatusReport = await sails.helpers.http.get('https://www.mergefreeze.com/api/branches/fleetdm/fleet/main', { access_token: sails.config.custom.mergeFreezeAccessToken }) //eslint-disable-line camelcase
@ -413,25 +409,42 @@ module.exports = {
sails.log.verbose('#'+prNumber+' is under consideration... The MergeFreeze API claims that it current main branch "frozen" status is:',mergeFreezeMainBranchStatusReport.frozen);
let isMainBranchFrozen = mergeFreezeMainBranchStatusReport.frozen;
// Add the #handbook label to PRs that only make changes to the handbook.
// Add the #handbook label to PRs that only make changes to the handbook,
// and remove it from PRs that NO LONGER ONLY contain changes to the handbook.
let isHandbookPR = false;
if(repo === 'fleet'){
isHandbookPR = await sails.helpers.githubAutomations.getIsPrOnlyHandbookChanges.with({prNumber: prNumber});
}//fi
if(isHandbookPR) {
// [?] https://docs.github.com/en/rest/issues/labels#add-labels-to-an-issue
await sails.helpers.http.post(`https://api.github.com/repos/${owner}/${repo}/issues/${prNumber}/labels`, {
labels: ['#handbook']
}, baseHeaders);
} else if (existingLabels.includes('#handbook')) {
// [?] https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#remove-a-label-from-an-issue
await sails.helpers.http.del(`https://api.github.com/repos/${owner}/${repo}/issues/${prNumber}/labels/${encodeURIComponent('#handbook')}`, {}, baseHeaders);
}//fi
// Add the appropriate label to PRs awaiting review from the CEO so that these PRs show up in kanban.
// [?] https://docs.github.com/en/webhooks-and-events/webhooks/webhook-events-and-payloads?actionType=edited#pull_request
let isAwaitingCeoReview = _.isArray(pr.requested_reviewers) && _.pluck(pr.requested_reviewers, 'login').includes('mikermcneil');
let isAwaitingCeoReview = alreadyRequestedReviewers.includes('mikermcneil');
if (isAwaitingCeoReview) {
// [?] https://docs.github.com/en/rest/issues/labels#add-labels-to-an-issue
await sails.helpers.http.post(`https://api.github.com/repos/${owner}/${repo}/issues/${prNumber}/labels`, {
labels: ['#g-ceo']
}, baseHeaders);
} else if (existingLabels.includes('#g-ceo')) {
// [?] https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#remove-a-label-from-an-issue
await sails.helpers.http.del(`https://api.github.com/repos/${owner}/${repo}/issues/${prNumber}/labels/${encodeURIComponent('#handbook')}`, {}, baseHeaders);
}//fi
// Now, if appropriate, auto-approve the change.
let isAutoApproved = await sails.helpers.githubAutomations.getIsPrPreapproved.with({
repo: repo,
prNumber: prNumber,
githubUserToCheck: sender.login,
isGithubUserMaintainerOrDoesntMatter: GITHUB_USERNAMES_OF_BOTS_AND_MAINTAINERS.includes(sender.login.toLowerCase())
});
if (isAutoApproved) {
// [?] https://docs.github.com/en/rest/reference/pulls#create-a-review-for-a-pull-request
await sails.helpers.http.post(`https://api.github.com/repos/${owner}/${repo}/pulls/${prNumber}/reviews`, {

View file

@ -109,29 +109,25 @@ module.exports.custom = {
// ````
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Code for core product and integrations
// 🚀 Code for core product and integrations
'ee/tools/puppet': 'georgekarrv',//« Puppet integration (especially useful with macOS MDM turned on) -- FYI: Originally developed by request from "customer-eponym"
// Reference, config surface, built-in queries, API, and other documentation
'docs': 'rachaelshaw', //« Fleet documentation (who is auto-requested as reviewer for changes to docs?)
'docs/Using-Fleet/REST-API.md': 'rachaelshaw', //« REST API reference documentation
'docs/Contributing/API-for-contributors.md': 'rachaelshaw',//« Advanced / contributors-only API reference documentation
'schema': 'rachaelshaw', //« Data tables (osquery/fleetd schema) documentation
'docs/01-Using-Fleet/standard-query-library/standard-query-library.yml': 'zwass', //« Built-in queries
// ⚗️ Reference, config surface, built-in queries, API, and other documentation
// 'docs': '', // « Covered in CODEOWNERS (2023-07-22)
// 'docs/Using-Fleet/REST-API.md': '', // « Covered in CODEOWNERS (2023-07-22)
// 'docs/Contributing/API-for-contributors.md': '', // « Covered in CODEOWNERS (2023-07-22)
// 'schema': '', // « Covered in CODEOWNERS (2023-07-22)
'docs/01-Using-Fleet/standard-query-library/standard-query-library.yml': 'rachaelshaw', //« Built-in queries
'ee/cis': 'sharon-fdm',//« Fleet Premium only: built-in queries (built-in policies for CIS benchmarks) -- FYI: On 2023-07-15, we changed this so that Sharon, Lucas, Marcos, and Rachel are all maintainers, but where there is a single DRI who is automatically requested approval from.
// Articles and release notes
// 🫧 Articles and release notes
'articles': 'jarodreyes',
'CHANGELOG.md': 'lukeheath',
// Website (fleetdm.com)
// 🫧 Website (fleetdm.com)
'website': 'mikermcneil',// (catch-all)
'website/assets': 'eashaw', // « Eric is DRI for website frontend code
'website/views': 'eashaw',
'website/views/pages/pricing.ejs': 'mikermcneil',//« But CEO is DRI for pricing
'website/api': 'mikermcneil',//« Website backend, scripts, deps
'website/api/controllers/webhooks/receive-from-github.js': 'mikermcneil',// github bot (webhook)
'website/api/controllers/imagine': 'eashaw',// landing pages
@ -140,25 +136,27 @@ module.exports.custom = {
'website/scripts': 'mikermcneil',
'website/package.json': 'eashaw',
// Other brandfronts
// 🫧 Pricing and features
// 'website/views/pages/pricing.ejs': '', // « Covered in CODEOWNERS (2023-07-22)
// 'handbook/product/pricing-features-table.yml': '', // « Covered in CODEOWNERS (2023-07-22)
// 🫧 Other brandfronts
'README.md': 'mikermcneil',// « GitHub brandfront
'tools/fleetctl-npm/README.md': 'mikermcneil',// « NPM brandfront (npmjs.com/package/fleetctl)
// Repo automation and change control settings
// 🦿 Repo automation and change control settings
'CODEOWNERS': 'mikermcneil',
'website/config/custom.js': 'mikermcneil',
// Handbook
'handbook/company': 'mikermcneil',
// 🦿 Handbook
'handbook/company/ceo-handbook.md': 'sampfluger88',
'handbook/business-operations': 'mikermcneil',// TODO: Transfer to joanne once the philosophical stuff is deduplicated
'handbook/engineering': 'lukeheath',
'handbook/product': 'zhumo',
'handbook/product/pricing-features-table.yml': 'mikermcneil',//« CEO is current DRI for product marketing (e.g. features table)
'handbook/customers': 'alexmitchelliii',
'handbook/marketing': 'jarodreyes',
'handbook/README.md': 'mikermcneil', // « This is the "Table of contents" landing page
// GitHub issue templates
// 🦿 GitHub issue templates
'.github/ISSUE_TEMPLATE': 'mikermcneil',//« GitHub issue templates
},
@ -194,8 +192,8 @@ module.exports.custom = {
// Articles and release notes
'CHANGELOG.md': ['zwass', 'mikermcneil', 'spokanemac', 'noahtalerman', 'zhumo', 'lukeheath'],
'articles': ['jarodreyes', 'mike-j-thomas', 'eashaw', 'zwass', 'mikermcneil'],
'website/assets/images/articles': ['jarodreyes', 'mike-j-thomas', 'eashaw', 'zwass', 'mikermcneil'],
'articles': ['jarodreyes', 'mike-j-thomas', 'eashaw', 'zwass', 'mikermcneil', 'spokanemac'],
'website/assets/images/articles': ['spokanemac', 'jarodreyes', 'mike-j-thomas', 'eashaw', 'zwass', 'mikermcneil'],
// Website (fleetdm.com)
'website': 'mikermcneil',// (default for website)
@ -250,7 +248,7 @@ module.exports.custom = {
'.github/workflows': ['mikermcneil', 'zwass', 'hollidayn', 'lukeheath'],//« CI/CD workflows
// Repo automation and change control settings
'CODEOWNERS': ['mikermcneil'],
'CODEOWNERS': ['mikermcneil', 'zwass'],
'.gitignore': ['mikermcneil', 'zwass', 'hollidayn', 'dherder', 'zayhanlon', 'lukeheath', 'zwinnerman-fleetdm', 'rfairburn'],// « what files should not be checked in?
'free-for-all': '*',//« Folder that any fleetie (core team member, not consultants) can push to, willy-nilly