Skip to content

Commit 4ca8912

Browse files
klutchellclaude
klutchell
and
claude
committed
refactor(variables): align with Diffable contract pattern
Refactor Variables plugin to match the single-item Diffable contract used by labels, milestones, and other plugins. The previous update() reimplemented sync() logic internally; now each method handles one item and lets Diffable.sync() orchestrate iteration. - Simplify update() from 90-line array-diffing to single-item PATCH - Simplify changed() from JSON.stringify comparison to value check - Remove getChanged(), lodash dependency, .then(res=>res) no-ops - Match labels.js nop return pattern: Promise.resolve([NopCommand]) - Fix inconsistent toUpperCase() between add/remove/update - Let errors propagate to Diffable.sync() instead of swallowing - Normalize find() to strip API metadata fields (created_at, etc.) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Kyle Harding <kyle@balena.io>
1 parent c519148 commit 4ca8912

File tree

2 files changed

+172
-417
lines changed

2 files changed

+172
-417
lines changed

‎lib/plugins/variables.js‎

Lines changed: 34 additions & 213 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
const _ = require('lodash')
21
const Diffable = require('./diffable')
32
const NopCommand = require('../nopcommand')
43

@@ -7,244 +6,66 @@ module.exports = class Variables extends Diffable {
76
super(...args)
87

98
if (this.entries) {
10-
// Force all names to uppercase to avoid comparison issues.
119
this.entries.forEach((variable) => {
1210
variable.name = variable.name.toUpperCase()
1311
})
1412
}
1513
}
1614

17-
/**
18-
* Look-up existing variables for a given repository
19-
*
20-
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#list-repository-variables} list repository variables
21-
* @returns {Array.<object>} Returns a list of variables that exist in a repository
22-
*/
23-
async find () {
15+
find () {
2416
this.log.debug(`Finding repo vars for ${this.repo.owner}/${this.repo.repo}`)
25-
const { data: { variables } } = await this.github.request('GET /repos/:org/:repo/actions/variables', {
17+
return this.github.request('GET /repos/:org/:repo/actions/variables', {
2618
org: this.repo.owner,
2719
repo: this.repo.repo
28-
})
29-
return variables.map(({ name, value }) => ({ name, value }))
30-
}
31-
32-
/**
33-
* Compare the existing variables with what we've defined as code
34-
*
35-
* @param {Array.<object>} existing Existing variables defined in the repository
36-
* @param {Array.<object>} variables Variables that we have defined as code
37-
*
38-
* @returns {object} The results of a list comparison
39-
*/
40-
getChanged (existing, variables = []) {
41-
const result =
42-
JSON.stringify(
43-
existing.sort((x1, x2) => {
44-
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
45-
})
46-
) !==
47-
JSON.stringify(
48-
variables.sort((x1, x2) => {
49-
return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase())
50-
})
51-
)
52-
return result
20+
}).then(({ data: { variables } }) => variables.map(({ name, value }) => ({ name, value })))
5321
}
5422

55-
/**
56-
* Compare existing variables with what's defined
57-
*
58-
* @param {Object} existing The existing entries in GitHub
59-
* @param {Object} attrs The entries defined as code
60-
*
61-
* @returns
62-
*/
6323
comparator (existing, attrs) {
6424
return existing.name === attrs.name
6525
}
6626

67-
/**
68-
* Return a list of changed entries
69-
*
70-
* @param {Object} existing The existing entries in GitHub
71-
* @param {Object} attrs The entries defined as code
72-
*
73-
* @returns
74-
*/
7527
changed (existing, attrs) {
76-
return this.getChanged(_.castArray(existing), _.castArray(attrs))
28+
return existing.value !== attrs.value
7729
}
7830

79-
/**
80-
* Update an existing variable if the value has changed
81-
*
82-
* @param {Array.<object>} existing Existing variables defined in the repository
83-
* @param {Array.<object>} variables Variables that we have defined as code
84-
*
85-
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#update-a-repository-variable} update a repository variable
86-
* @returns
87-
*/
88-
async update (existing, variables = []) {
89-
this.log.debug(`Updating a repo var existing params ${JSON.stringify(existing)} and new ${JSON.stringify(variables)}`)
90-
existing = _.castArray(existing)
91-
variables = _.castArray(variables)
92-
const changed = this.getChanged(existing, variables)
93-
94-
if (changed) {
95-
const nopCommands = []
96-
let existingVariables = [...existing]
97-
98-
for (const variable of variables) {
99-
const existingVariable = existingVariables.find((_var) => _var.name === variable.name)
100-
if (existingVariable) {
101-
existingVariables = existingVariables.filter((_var) => _var.name !== variable.name)
102-
if (existingVariable.value !== variable.value) {
103-
if (this.nop) {
104-
nopCommands.push(new NopCommand(
105-
this.constructor.name,
106-
this.repo,
107-
null,
108-
`Update variable ${variable.name}`
109-
))
110-
} else {
111-
await this.github
112-
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
113-
org: this.repo.owner,
114-
repo: this.repo.repo,
115-
variable_name: variable.name.toUpperCase(),
116-
value: variable.value.toString()
117-
})
118-
.then((res) => {
119-
return res
120-
})
121-
.catch((e) => {
122-
this.logError(e)
123-
})
124-
}
125-
}
126-
} else {
127-
if (this.nop) {
128-
nopCommands.push(new NopCommand(
129-
this.constructor.name,
130-
this.repo,
131-
null,
132-
`Add variable ${variable.name}`
133-
))
134-
} else {
135-
await this.github
136-
.request('POST /repos/:org/:repo/actions/variables', {
137-
org: this.repo.owner,
138-
repo: this.repo.repo,
139-
name: variable.name.toUpperCase(),
140-
value: variable.value.toString()
141-
})
142-
.then((res) => {
143-
return res
144-
})
145-
.catch((e) => {
146-
this.logError(e)
147-
})
148-
}
149-
}
150-
}
151-
152-
for (const variable of existingVariables) {
153-
if (this.nop) {
154-
nopCommands.push(new NopCommand(
155-
this.constructor.name,
156-
this.repo,
157-
null,
158-
`Remove variable ${variable.name}`
159-
))
160-
} else {
161-
await this.github
162-
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
163-
org: this.repo.owner,
164-
repo: this.repo.repo,
165-
variable_name: variable.name.toUpperCase()
166-
})
167-
.then((res) => {
168-
return res
169-
})
170-
.catch((e) => {
171-
this.logError(e)
172-
})
173-
}
174-
}
175-
176-
if (this.nop) {
177-
return nopCommands.length === 1 ? nopCommands[0] : nopCommands
178-
}
31+
update (existing, attrs) {
32+
if (this.nop) {
33+
return Promise.resolve([
34+
new NopCommand(this.constructor.name, this.repo, null, `Update variable ${attrs.name}`)
35+
])
17936
}
37+
return this.github.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
38+
org: this.repo.owner,
39+
repo: this.repo.repo,
40+
variable_name: attrs.name.toUpperCase(),
41+
value: attrs.value.toString()
42+
})
18043
}
18144

182-
/**
183-
* Add a new variable to a given repository
184-
*
185-
* @param {object} variable The variable to add, with name and value
186-
*
187-
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-a-repository-variable} create a repository variable
188-
* @returns
189-
*/
190-
async add (variable) {
191-
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
192-
45+
add (attrs) {
19346
if (this.nop) {
194-
return new NopCommand(
195-
this.constructor.name,
196-
this.repo,
197-
null,
198-
`Add variable ${variable.name}`
199-
)
47+
return Promise.resolve([
48+
new NopCommand(this.constructor.name, this.repo, null, `Add variable ${attrs.name}`)
49+
])
20050
}
201-
202-
await this.github
203-
.request('POST /repos/:org/:repo/actions/variables', {
204-
org: this.repo.owner,
205-
repo: this.repo.repo,
206-
name: variable.name,
207-
value: variable.value.toString()
208-
})
209-
.then((res) => {
210-
return res
211-
})
212-
.catch((e) => {
213-
this.logError(e)
214-
})
51+
return this.github.request('POST /repos/:org/:repo/actions/variables', {
52+
org: this.repo.owner,
53+
repo: this.repo.repo,
54+
name: attrs.name.toUpperCase(),
55+
value: attrs.value.toString()
56+
})
21557
}
21658

217-
/**
218-
* Remove variables that aren't defined as code
219-
*
220-
* @param {String} existing Name of the existing variable to remove
221-
*
222-
* @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#delete-a-repository-variable} delete a repository variable
223-
* @returns
224-
*/
225-
async remove (existing) {
226-
this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`)
227-
59+
remove (existing) {
22860
if (this.nop) {
229-
return new NopCommand(
230-
this.constructor.name,
231-
this.repo,
232-
null,
233-
`Remove variable ${existing.name}`
234-
)
61+
return Promise.resolve([
62+
new NopCommand(this.constructor.name, this.repo, null, `Remove variable ${existing.name}`)
63+
])
23564
}
236-
237-
await this.github
238-
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
239-
org: this.repo.owner,
240-
repo: this.repo.repo,
241-
variable_name: existing.name
242-
})
243-
.then((res) => {
244-
return res
245-
})
246-
.catch((e) => {
247-
this.logError(e)
248-
})
65+
return this.github.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
66+
org: this.repo.owner,
67+
repo: this.repo.repo,
68+
variable_name: existing.name.toUpperCase()
69+
})
24970
}
25071
}

0 commit comments

Comments
 (0)