Skip to content

Templating: don't check the users who posted reviews#256

Open
trexfeathers wants to merge 3 commits intoSciTools:mainfrom
trexfeathers:templating-dont-check-user
Open

Templating: don't check the users who posted reviews#256
trexfeathers wants to merge 3 commits intoSciTools:mainfrom
trexfeathers:templating-dont-check-user

Conversation

@trexfeathers
Copy link
Contributor

gh api user is not available when running as part of CI. Plus some information states scitools-ci and other information states scitools-ci[bot], so it's difficult to program this reliably without a lot of testing.

In reality: any review which begins with the Templating level 2 heading is likely to be the appropriate one to modify, so I propose not bothering to check the user at all.

@trexfeathers trexfeathers requested a review from HGWright February 5, 2026 12:45
Copy link
Contributor

@HGWright HGWright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have some hidden text, so that we can go from "unlikely" that someone accidentally posts a review like that, to "near impossible" that someone could accidentally match the format this check looks for?

@trexfeathers
Copy link
Contributor Author

trexfeathers commented Feb 5, 2026

Would it be possible to have some hidden text, so that we can go from "unlikely" that someone accidentally posts a review like that, to "near impossible" that someone could accidentally match the format this check looks for?

Good idea - 28d988e. I've edited this comment to conform, as an easy way to test that this works: SciTools/python-stratify#404 (review)

After merging this PR: if we re-run the CI on SciTools/python-stratify#404 we should see the "test" disappear from the end when the automation overwrites the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants