-
Notifications
You must be signed in to change notification settings - Fork 212
Ignore pinned issues #266
base: master
Are you sure you want to change the base?
Ignore pinned issues #266
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,7 +62,7 @@ module.exports = class Stale { | |
} | ||
} | ||
|
||
getStale (type) { | ||
async getStale (type) { | ||
const onlyLabels = this.getConfigValue(type, 'onlyLabels') | ||
const staleLabel = this.getConfigValue(type, 'staleLabel') | ||
const exemptLabels = this.getConfigValue(type, 'exemptLabels') | ||
|
@@ -80,7 +80,68 @@ module.exports = class Stale { | |
|
||
const query = queryParts.join(' ') | ||
const days = this.getConfigValue(type, 'days') || this.getConfigValue(type, 'daysUntilStale') | ||
return this.search(type, days, query) | ||
const results = await this.search(type, days, query) | ||
|
||
const pinned = await this.getPinnedNumbers(type) | ||
const nonPinned = results.data.items.filter(issue => pinned.includes(issue.number)) | ||
return { | ||
...results, | ||
data: { | ||
...results.data, | ||
total_count: nonPinned.length, | ||
items: nonPinned, | ||
} | ||
} | ||
} | ||
|
||
async getPinnedNumbers(type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could potentially add a config setting to control if pinned issues should be ignored or not, but I wasn't sure if it was worth it. |
||
if (type === 'pulls') { | ||
// Pull Requests cannot be pinned | ||
return [] | ||
} else if (type !== 'issues') { | ||
throw new Error(`Unknown type: ${type}. Valid types are 'pulls' and 'issues'`) | ||
} | ||
|
||
const { owner, repo } = this.config | ||
|
||
try { | ||
// GitHub's v3 REST API doesn't support Pinned Issues; v4 GraphQL API does | ||
const {data, errors} = await this.github.graphql( | ||
` | ||
query Issues($owner: String!, $repo: String!) { | ||
repository(owner: $owner, name: $repo) { | ||
pinnedIssues(first: 100) { | ||
nodes { | ||
number | ||
} | ||
pageInfo { | ||
# This should always be false, as only 3 issues can be pinned at the same time | ||
hasNextPage | ||
} | ||
Comment on lines
+117
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could go away, but I felt it was was useful to document why we don't have to paginate. |
||
} | ||
} | ||
} | ||
`, | ||
Comment on lines
+110
to
+124
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally extracted this into a constant, but decided it might be clearer to have it nearby. |
||
{ | ||
owner, | ||
repo, | ||
headers: { | ||
// Opt-in to Pinned Issues API preview | ||
accept: 'application/vnd.github.elektra-preview+json' | ||
}, | ||
}, | ||
) | ||
|
||
if (errors && errors.length) { | ||
throw new Error(errors[0].message) | ||
} | ||
|
||
return data.repository.pinnedIssues.nodes.map(issue => issue.number) | ||
} catch (error) { | ||
this.logger.error(`Encountered an error while excluding pinned items for ${owner}/${repo}: ${error}`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't sure if this was the best way to log errors, but I think it's important that the bot not break if the |
||
// In the event of an error, proceed as if no pinned issues found | ||
return [] | ||
} | ||
} | ||
|
||
getClosable (type) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opted to preserve any untouched portions of the return payload, rather than just return
{data:{items: nonPinned}}
, despite the fact they aren't used.It felt weird to change the return type, rather than just the contents.