Skip to content

Add documentation for new ApPipeWithFakes#241

Merged
BrunoSanchez merged 2 commits intomainfrom
tickets/DM-52361
Sep 10, 2025
Merged

Add documentation for new ApPipeWithFakes#241
BrunoSanchez merged 2 commits intomainfrom
tickets/DM-52361

Conversation

@BrunoSanchez
Copy link
Member

This relates to changes made in DM-48896

@kfindeisen
Copy link
Member

FTR, the DM-48896 changes are #223.

Copy link
Member

@kfindeisen kfindeisen left a comment

Choose a reason for hiding this comment

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

While the Sphinx docs are impressive and helpful, what I actually asked for was a comment in <instrument>/ApPipeWithFakes.yaml saying that the cross-reference to _instruments/ApPipeWithFakes.yaml is to a generated file. That's where it actually needs to be documented to keep from being overlooked by someone looking through or editing the pipelines.

If you do keep the Sphinx docs, please make sure they're organized in a modular, topic-centric form that keeps each page focused and defers to related docs wherever possible. You may find DMTN-030 helpful if you're unfamiliar with this style. In particular, pipeline-with-fakes.rst is (re-?)documenting a lot of things that are really about the source injection framework, and it felt like it couldn't decide whether it's written for people running the pipeline or those working on ap_pipe itself.

Any time you are making documentation changes, especially to Sphinx, please build the documentation and proofread it. You can catch a lot of errors just by looking at the rendered HTML.

Copy link
Member

@kfindeisen kfindeisen left a comment

Choose a reason for hiding this comment

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

Looks good!

@BrunoSanchez BrunoSanchez merged commit cfc634d into main Sep 10, 2025
2 checks passed
@BrunoSanchez BrunoSanchez deleted the tickets/DM-52361 branch September 10, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants