Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: standardize variable file naming and merge glob patterns #732

Open
wants to merge 4 commits into
base: main-enterprise
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
if (nop) {
let filename = env.SETTINGS_FILE_PATH
if (!deploymentConfig) {
filename = env.DEPLOYMENT_CONFIG_FILE
filename = env.DEPLOYMENT_CONFIG_FILE_PATH
deploymentConfig = {}
}
const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
Expand All @@ -53,7 +53,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
if (nop) {
let filename = env.SETTINGS_FILE_PATH
if (!deploymentConfig) {
filename = env.DEPLOYMENT_CONFIG_FILE
filename = env.DEPLOYMENT_CONFIG_FILE_PATH
deploymentConfig = {}
}
const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
Expand All @@ -78,7 +78,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
if (nop) {
let filename = env.SETTINGS_FILE_PATH
if (!deploymentConfig) {
filename = env.DEPLOYMENT_CONFIG_FILE
filename = env.DEPLOYMENT_CONFIG_FILE_PATH
deploymentConfig = {}
}
const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
Expand All @@ -104,7 +104,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
if (nop) {
let filename = env.SETTINGS_FILE_PATH
if (!deploymentConfig) {
filename = env.DEPLOYMENT_CONFIG_FILE
filename = env.DEPLOYMENT_CONFIG_FILE_PATH
deploymentConfig = {}
}
const nopcommand = new NopCommand(filename, repo, null, e, 'ERROR')
Expand All @@ -123,7 +123,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
*/
async function loadYamlFileSystem () {
if (deploymentConfig === undefined) {
const deploymentConfigPath = env.DEPLOYMENT_CONFIG_FILE
const deploymentConfigPath = env.DEPLOYMENT_CONFIG_FILE_PATH
if (fs.existsSync(deploymentConfigPath)) {
deploymentConfig = yaml.load(fs.readFileSync(deploymentConfigPath))
} else {
Expand All @@ -134,7 +134,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}

function getAllChangedSubOrgConfigs (payload) {
const settingPattern = new Glob(`${env.CONFIG_PATH}/suborgs/*.yml`)
const settingPattern = Settings.SUB_ORG_PATTERN
// Changes will be an array of files that were added
const added = payload.commits.map(c => {
return (c.added.filter(s => {
Expand All @@ -158,7 +158,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}

function getAllChangedRepoConfigs (payload, owner) {
const settingPattern = new Glob(`${env.CONFIG_PATH}/repos/*.yml`)
const settingPattern = Settings.REPO_PATTERN
// Changes will be an array of files that were added
const added = payload.commits.map(c => {
return (c.added.filter(s => {
Expand Down Expand Up @@ -270,11 +270,11 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}

const settingsModified = payload.commits.find(commit => {
return commit.added.includes(Settings.FILE_NAME) ||
commit.modified.includes(Settings.FILE_NAME)
return commit.added.includes(Settings.FILE_PATH) ||
commit.modified.includes(Settings.FILE_PATH)
})
if (settingsModified) {
robot.log.debug(`Changes in '${Settings.FILE_NAME}' detected, doing a full synch...`)
robot.log.debug(`Changes in '${Settings.FILE_PATH}' detected, doing a full synch...`)
return syncAllSettings(false, context)
}

Expand All @@ -292,7 +292,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
}))
}

robot.log.debug(`No changes in '${Settings.FILE_NAME}' detected, returning...`)
robot.log.debug(`No changes in '${Settings.FILE_PATH}' detected, returning...`)
})

robot.on('create', async context => {
Expand Down Expand Up @@ -597,21 +597,21 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) =>
const changes = await context.octokit.repos.compareCommitsWithBasehead(params)
const files = changes.data.files.map(f => { return f.filename })

const settingsModified = files.includes(Settings.FILE_NAME)
const settingsModified = files.includes(Settings.FILE_PATH)

if (settingsModified) {
robot.log.debug(`Changes in '${Settings.FILE_NAME}' detected, doing a full synch...`)
robot.log.debug(`Changes in '${Settings.FILE_PATH}' detected, doing a full synch...`)
return syncAllSettings(true, context, context.repo(), pull_request.head.ref)
}

const repoChanges = getChangedRepoConfigName(new Glob(`${env.CONFIG_PATH}/repos/*.yml`), files, context.repo().owner)
const repoChanges = getChangedRepoConfigName(Settings.REPO_PATTERN, files, context.repo().owner)
if (repoChanges.length > 0) {
return Promise.all(repoChanges.map(repo => {
return syncSettings(true, context, repo, pull_request.head.ref)
}))
}

const subOrgChanges = getChangedSubOrgConfigName(new Glob(`${env.CONFIG_PATH}/suborgs/*.yml`), files, context.repo().owner)
const subOrgChanges = getChangedSubOrgConfigName(Settings.SUB_ORG_PATTERN, files, context.repo().owner)
if (subOrgChanges.length) {
return Promise.all(subOrgChanges.map(suborg => {
return syncSubOrgSettings(true, context, suborg, context.repo(), pull_request.head.ref)
Expand Down
2 changes: 1 addition & 1 deletion lib/deploymentConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class DeploymentConfig {
static overridevalidators = {}

static {
const deploymentConfigPath = process.env.DEPLOYMENT_CONFIG_FILE ? process.env.DEPLOYMENT_CONFIG_FILE : 'deployment-settings.yml'
const deploymentConfigPath = env.DEPLOYMENT_CONFIG_FILE_PATH
if (fs.existsSync(deploymentConfigPath)) {
this.config = yaml.load(fs.readFileSync(deploymentConfigPath))
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module.exports = {
ADMIN_REPO: process.env.ADMIN_REPO || 'admin',
CONFIG_PATH: process.env.CONFIG_PATH || '.github',
SETTINGS_FILE_PATH: process.env.SETTINGS_FILE_PATH || 'settings.yml',
DEPLOYMENT_CONFIG_FILE: process.env.DEPLOYMENT_CONFIG_FILE || 'deployment-settings.yml',
DEPLOYMENT_CONFIG_FILE_PATH: process.env.DEPLOYMENT_CONFIG_FILE || 'deployment-settings.yml',
CREATE_PR_COMMENT: process.env.CREATE_PR_COMMENT || 'true',
CREATE_ERROR_ISSUE: process.env.CREATE_ERROR_ISSUE || 'true',
BLOCK_REPO_RENAME_BY_HUMAN: process.env.BLOCK_REPO_RENAME_BY_HUMAN || 'false'
Expand Down
8 changes: 5 additions & 3 deletions lib/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ ${this.results.reduce((x, y) => {
}
)) {
delete subOrgConfigs[key]
}
}
}
}
return subOrgConfigs
Expand Down Expand Up @@ -857,7 +857,7 @@ ${this.results.reduce((x, y) => {
if (this.nop) {
//Remove nulls and undefined from the results
const results = res.flat(3).filter(r => r)

this.results = this.results.concat(results)
}
}
Expand Down Expand Up @@ -899,7 +899,9 @@ function prettify (obj) {
return JSON.stringify(obj, null, 2).replaceAll('\n', '<br>').replaceAll(' ', '&nbsp;')
}

Settings.FILE_NAME = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH)
Settings.FILE_PATH = path.posix.join(CONFIG_PATH, env.SETTINGS_FILE_PATH)
Settings.SUB_ORG_PATTERN = new Glob(`${CONFIG_PATH}/suborgs/*.yml`)
Settings.REPO_PATTERN = new Glob(`${CONFIG_PATH}/repos/*.yml`)

Settings.PLUGINS = {
repository: require('./plugins/repository'),
Expand Down
2 changes: 1 addition & 1 deletion test/integration/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function buildPushEvent () {
payload: {
ref: 'refs/heads/master',
repository,
commits: [{ modified: [settings.FILE_NAME], added: [] }]
commits: [{ modified: [settings.FILE_PATH], added: [] }]
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/plugins/collaborators.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('collaborators plugin', function () {
const configFile = Buffer.from(fs.readFileSync(pathToConfig, 'utf8'))
const encodedConfig = configFile.toString('base64')
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(OK, { content: encodedConfig, name: 'settings.yml', type: 'file' })
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/collaborators?affiliation=direct`)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/plugins/milestones.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('milestones plugin', function () {
const configFile = Buffer.from(fs.readFileSync(pathToConfig, 'utf8'))
const encodedConfig = configFile.toString('base64')
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(OK, { content: encodedConfig, name: 'settings.yml', type: 'file' })
githubScope
.patch(`/repos/${repository.owner.name}/${repository.name}`)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/plugins/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('repository plugin', function () {
const config = yaml.safeLoad(configFile, 'utf8')
const encodedConfig = configFile.toString('base64')
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(200, { content: encodedConfig, name: 'settings.yml', type: 'file' })
githubScope
.patch(`/repos/${repository.owner.name}/${repository.name}`, body => {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/plugins/teams.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('teams plugin', function () {
const greenkeeperKeeperTeamId = any.integer()
const formationTeamId = any.integer()
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(OK, { content: encodedConfig, name: 'settings.yml', type: 'file' })
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/teams`)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/triggers/push.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('push trigger', function () {
payload: {
ref: 'refs/heads/wip',
repository,
commits: [{ modified: [settings.FILE_NAME], added: [] }]
commits: [{ modified: [settings.FILE_PATH], added: [] }]
}
})
})
Expand Down
4 changes: 2 additions & 2 deletions test/integration/triggers/repository-created.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ describe('repository.created trigger', function () {

it('does not apply configuration when the repository does not have a settings.yml', async () => {
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(NOT_FOUND, {
message: 'Not Found',
documentation_url: 'https://developer.github.com/v3/repos/contents/#get-contents'
})
githubScope
.get(`/repos/${repository.owner.name}/.github/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/.github/contents/${settings.FILE_PATH}`)
.reply(NOT_FOUND, {
message: 'Not Found',
documentation_url: 'https://developer.github.com/v3/repos/contents/#get-contents'
Expand Down
4 changes: 2 additions & 2 deletions test/integration/triggers/repository-edited.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ describe('repository.edited trigger', function () {

it('does not apply configuration when the repository does not have a settings.yml', async () => {
githubScope
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/${repository.name}/contents/${settings.FILE_PATH}`)
.reply(NOT_FOUND, {
message: 'Not Found',
documentation_url: 'https://developer.github.com/v3/repos/contents/#get-contents'
})
githubScope
.get(`/repos/${repository.owner.name}/.github/contents/${settings.FILE_NAME}`)
.get(`/repos/${repository.owner.name}/.github/contents/${settings.FILE_PATH}`)
.reply(NOT_FOUND, {
message: 'Not Found',
documentation_url: 'https://developer.github.com/v3/repos/contents/#get-contents'
Expand Down
8 changes: 4 additions & 4 deletions test/unit/lib/env.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('env', () => {
expect(SETTINGS_FILE_PATH).toEqual('settings.yml')
})

it('loads default DEPLOYMENT_CONFIG_FILE if not passed', () => {
const SETTINGS_FILE_PATH = envTest.DEPLOYMENT_CONFIG_FILE
it('loads default DEPLOYMENT_CONFIG_FILE_PATH if not passed', () => {
const SETTINGS_FILE_PATH = envTest.DEPLOYMENT_CONFIG_FILE_PATH
expect(SETTINGS_FILE_PATH).toEqual('deployment-settings.yml')
})

Expand Down Expand Up @@ -47,8 +47,8 @@ describe('env', () => {
expect(CONFIG_PATH).toEqual('.config')
const SETTINGS_FILE_PATH = envTest.SETTINGS_FILE_PATH
expect(SETTINGS_FILE_PATH).toEqual('safe-settings.yml')
const DEPLOYMENT_CONFIG_FILE = envTest.DEPLOYMENT_CONFIG_FILE
expect(DEPLOYMENT_CONFIG_FILE).toEqual('safe-settings-deployment.yml')
const DEPLOYMENT_CONFIG_FILE_PATH = envTest.DEPLOYMENT_CONFIG_FILE_PATH
expect(DEPLOYMENT_CONFIG_FILE_PATH).toEqual('safe-settings-deployment.yml')
const CREATE_PR_COMMENT = envTest.CREATE_PR_COMMENT
expect(CREATE_PR_COMMENT).toEqual('false')
})
Expand Down