-
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
Conversation
} | ||
} | ||
|
||
async getPinnedNumbers(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.
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.
return { | ||
...results, | ||
data: { | ||
...results.data, | ||
total_count: nonPinned.length, | ||
items: nonPinned, | ||
} | ||
} |
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.
` | ||
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 | ||
} | ||
} | ||
} | ||
} | ||
`, |
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 originally extracted this into a constant, but decided it might be clearer to have it nearby.
pageInfo { | ||
# This should always be false, as only 3 issues can be pinned at the same time | ||
hasNextPage | ||
} |
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.
This could go away, but I felt it was was useful to document why we don't have to paginate.
|
||
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 comment
The 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 Pinned Issues Preview API
changes.
d292f86
to
d272cd6
Compare
This PR would address a current issue: https://stackoverflow.com/questions/62064833/github-probot-why-are-pinned-issues-treated-as-style-and-marked-as-wont-fix. The default config of ignoring a
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? |
This is still relevant and would be highly appreciated. |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? |
Context
Pinned Issues should never be considered stale, as they are obviously meant to stick around, or they wouldn't be pinned.
Currently, the bot treats Pinned Issues the same as any other issue. The workaround is to also add a label to these issues, to prevent the bot from flagging them.
What does this do?
This adds a filter step to the
getStale
function, which filters out Pinned Issues. This results in Pinned Issues never being marked as staleConsiderations
The GraphQL API is used to fetch the Pinned Issues, as the REST API does not support this functionality.
To access this information, the
Pinned Issues Preview
feature had to be enabled, via the specialAccept
header. This should be removed once the feature is released.In case the feature is removed or altered, causing an error, the code rescues errors and proceeds as if no issues are pinned.