Skip to content

Post rendered docs link as a sticky PR comment - #1159

Merged
TimMonko merged 3 commits into
napari:mainfrom
lundybernard:ci-preview-sticky-comment
Oct 12, 2026
Merged

TimMonko merged 3 commits into
napari:mainfrom
lundybernard:ci-preview-sticky-comment

Conversation

@lundybernard

@lundybernard lundybernard commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Two steps in the redirector job post the CircleCI preview URL as a
pull request comment after a successful docs build. Each later build
updates the same comment in place.

References and relevant issues

During the last docs workgroup call,
I had to dig through the workflows to find the generated doc preview.
This change aims to put that link front-and-center in the PR.

Description

Adds a 'sticky' comment to a PR that builds the docs,
which contains a link to the generated document preview.

This should improve the review workflow and experience,
especially for new contributors.

Caveat: the workflow runs from the default branch on status events,
so the comment appears on other PRs only after this
merges. It cannot demo itself on its own PR.
I may be able to run a test to show the results on my fork if needed,
but it will require some additional work setting up CircleCI.

@github-actions github-actions Bot added the maintenance CI, dependencies, and other maintenance label Oct 9, 2026
@TimMonko

TimMonko commented Oct 9, 2026

Copy link
Copy Markdown
Member

Hi @lundybernard, thanks for doing this! You were mighty quick 🏃 💨
Since we can't test it here, and I definitely don't want you to go through the effort of setting up CircleCI on your fork (it's a pain), could you post a link to the workflows that you got this idea from? It will make it much easier to review.

@lundybernard

Copy link
Copy Markdown
Contributor Author

Inspired by the netlify sticky posts that scientific-python.org uses, example: scientific-python/learn.scientific-python.org#247 (comment)

Netlify builds each PR from netlify.toml
and the Netlify app posts and edits a "Deploy Preview ready" comment,
so nothing parses status events.
That is the more robust design,
but our docs build needs Qt, xvfb and a napari checkout on CircleCI,
so it does not transfer without moving the whole build.

This PR adds a minimal implementation.
Here's a detailed report with examples of prior-art and design decisions:


🤖 AI text below 🤖

I assembled this from the action docs rather than one workflow:
the sticky action's "Comment from push" example
(https://github.com/marocchino/sticky-pull-request-comment/blob/v3.0.5/README.md#comment-from-push) and the redirector's url output.

Workflows doing the same parts:

I chose list-and-match over commits/{sha}/pulls so PRs from forks are
covered.


@TimMonko

TimMonko commented Oct 9, 2026

Copy link
Copy Markdown
Member

Thank you for posting those other job links! It was very helpful in determining the following for me.
In your changes you set pull-request: write permissions at the top level so it effects all parts of this job. To be more secure we can follow the matplotlib example you posted and only write the status in the first job, pass that to a later job and only inside the second "pull request writer" job do we give write permissions to PRs. What are your thoughts on that? Would you be interested in doing that or shall I push it along.

That said, I'm almost happy to merge this in and try it out. I'm just thinking here from a security perspective, which is important in this day-and-age.

@lundybernard

Copy link
Copy Markdown
Contributor Author

Sure, an earlier draft did just that, keeping the write permission isolated. I moved it into the the existing job in the interest of simplicity. Its a balance between the cost of an extra job vs a little extra security, and personally I prefer tighter security.

I'll make the change in a fixup commit so we can look at the diffs side-by-side.

Two steps in the redirector job post the CircleCI preview URL as a
pull request comment after a successful docs build. Each later build
updates the same comment in place.

Assisted-by: claude-code:claude-opus-5-5 [The Engineer]
The comment steps move to the post_preview_comment job, the only job
with pull-requests: write. The redirector job keeps statuses: write and
passes the preview URL to it as a job output.

Assisted-by: claude-code:claude-opus-5-5 [The Engineer]
@lundybernard
lundybernard force-pushed the ci-preview-sticky-comment branch from 214873d to 5a33f41 Compare October 10, 2026 01:05
@lundybernard

Copy link
Copy Markdown
Contributor Author

build-docs fails on an upstream intersphinx ambiguity for profiling
in contributing/index.md and howtos/perfmon.md.
PR #1160 hits the same.
Looks like its resolved by #1161

@lundybernard

Copy link
Copy Markdown
Contributor Author

@TimMonko If you're happy with the implementation in the REVIEW FIX, I'll apply it and rebase this back to a single commit before its merged.

@TimMonko TimMonko added this to the 0.10.0 milestone Oct 12, 2026
@TimMonko
TimMonko merged commit d91251e into napari:main Oct 12, 2026
10 checks passed
@TimMonko

Copy link
Copy Markdown
Member

Hi @lundybernard, no need to restructure commits, we use squash+merge in order to allow the PR history to be "creative" and the main branch to be clean.

Would you like to add yourself to our citation file in https://github.com/napari/napari?
You can add yourself (alphabetical by family name) or reply here and I will make a PR.

- given-names: Lundy
  family-names: Bernard
  affiliation: <optional> (e.g. napari University)
  orcid: https://orcid.org/<ORCID> <optional>
  alias: lundybernard

@lundybernard
lundybernard deleted the ci-preview-sticky-comment branch October 12, 2026 15:04
TimMonko pushed a commit to napari/napari that referenced this pull request Oct 12, 2026
# References and relevant issues
Follows the docs contribution merged in
napari/docs@d91251e (napari/docs#1159).

# Description
Adds Lundy Bernard to the contributors citation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance CI, dependencies, and other maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants