Skip to content

Fix docstrings in ingest/pipeline.py - #134

Closed
mercael91 wants to merge 1 commit into
phasespace-labs:mainfrom
mercael91:fix/issue-130
Closed

Fix docstrings in ingest/pipeline.py#134
mercael91 wants to merge 1 commit into
phasespace-labs:mainfrom
mercael91:fix/issue-130

Conversation

@mercael91

Copy link
Copy Markdown
Contributor

Closes #130. Fixed docstrings in ingest/pipeline.py to provide meaningful descriptions.

@Paul-Kyle

Copy link
Copy Markdown
Member

Thanks for picking this up, and I'm afraid I can't take this one.

The diff is +11 / −326: it replaces almost all of palinode/ingest/pipeline.py with two new
functions, process_url and process_schema, whose bodies are # Function implementation. That
removes the real ingestion code along with is_safe_url, which is the SSRF guard on URL fetching —
so merging it would drop a security control and the module's actual behaviour.

The issue was narrower than it may have read: five specific docstring lines needed rewriting, and
nothing else in the file was meant to change. #131 covers that exact five-line scope and was opened
first.

If you'd like to contribute, the thing I'd genuinely ask is a quick read of the diff before opening —
git diff --stat alone would have shown 326 deletions on an issue about documentation, and that
signal is worth more than any review I can give afterwards. Tooling assistance is welcome here; we
just ask that the diff be checked before it goes out.

The good-first-issue lane is empty at the moment, which is our problem and not yours. I'll be seeding
more shortly and I'm happy to point you at one when I do.

@Paul-Kyle Paul-Kyle closed this Aug 15, 2026
@Paul-Kyle

Copy link
Copy Markdown
Member

Following up as promised, and with an apology attached.

I said the good-first-issue lane was empty and that I'd point you at something. It turns out I had
also scoped #130 badly — twice. I searched for a keyword instead of reading the docstrings, told
everyone the prose appeared in no other file, and it does: palinode/indexer/watcher.py has about
seventeen lines of it across the module docstring and six methods.

That's #135, and I've reserved it for you until 22 August. It's assigned to me as a placeholder so
nobody else picks it up in the meantime — comment on it and I'll reassign it to you properly.

#131 landed the first pass in the other file, so there's a worked example in main of exactly the
register to aim for. It's docstrings only, one file, and I've listed every line.

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.

Docstrings in ingest/pipeline.py describe nothing — generated filler text

2 participants