Skip to content

Clarify nested-call exemptions and bare-name if() conditions - #18

Merged
dem-extra1 merged 3 commits into
mainfrom
claude/fix-preview-docx-tracked-changes-q8pv7r
Oct 12, 2026
Merged

dem-extra1 merged 3 commits into
mainfrom
claude/fix-preview-docx-tracked-changes-q8pv7r

Conversation

@dem-extra1

Copy link
Copy Markdown
Contributor

Before: avoid-nesting.qmd listed the arguments exempt from the nested-call rule (tidyselect helpers, data-masked expressions, aes(), formulas) without saying whether the list was closed. It also said if() conditions are "always a bare name", without saying whether that covers a plain comparison like if (x > 0).

After: the page says the test (does the outer function capture and interpret the argument?) decides the exemption, and the listed cases are examples. A worked case, select(df, all_of(names(x))), shows that the exemption stops at the captured argument. A single comparison like x > 0 is named as still needing a named condition.

How: two prose edits to chapters/coding-style/avoid-nesting.qmd. They settle the two open scope questions left after psc#15. Ezra handed both decisions over with daytb.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CVcpP4PWwqXdXBQtQF5JeK


Generated by Claude Code

The exemption list for arguments an outer function captures itself
read as possibly closed. State that the capture test decides the
exemption and the listed cases are examples, and that an argument
evaluated as an ordinary value is never exempt.

The bare-name rule for if()/while() conditions already covered
conditions with no nested call; name a single comparison like
`x > 0` explicitly so nobody reads it as an exception.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVcpP4PWwqXdXBQtQF5JeK
Address review: replace the ambiguous 'That test' with the criterion,
and give a concrete non-exempt case (an inner call inside all_of()).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVcpP4PWwqXdXBQtQF5JeK
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Address the review's optional note: the test restated the clause it
followed. Fold the not-a-closed-list point into that clause.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVcpP4PWwqXdXBQtQF5JeK

Copy link
Copy Markdown
Contributor Author

link-checker / link-checker failed on a49c0e9 because github.com returned a 503 for one kmishra9/Flu-Absenteeism link in chapters/coding-style/comments.qmd. This PR does not touch that file; it only edits chapters/coding-style/avoid-nesting.qmd. The link checker hit the same github.com 503 class on psc#15 and psc#2 earlier today, and both cleared on a re-run. No fix exists to port for a transient upstream outage, so I am re-running the job once.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished review — View run


Review: #18

Range reviewed: 93c5f570..958c5d33 (commit a49c0e9, "Drop the repeated exemption sentence"), which is 2 insertions / 3 deletions in chapters/coding-style/avoid-nesting.qmd.

What changed

This commit addresses, almost verbatim, the only non-blocking note from the prior review (commit 93c5f570): it collapsed the two circular sentences

nor an argument the outer function captures and interprets itself.
Whether the outer function captures and interprets an argument decides that exemption.
These cases are examples of it, not a closed list:

into one:

nor an argument the outer function captures and interprets itself,
as in these examples, which are not a closed list:

(chapters/coding-style/avoid-nesting.qmd:19-20)

Checks

  • Reads correctly and removes the restated-test redundancy flagged previously; no meaning is lost — the "not a closed list" qualifier and the capture/interpret test both survive, just stated once.
  • The surrounding comma-separated enumeration ({tidyselect} helpers, data-masked expressions, aes() mappings, formulas) is unchanged prose, not a markdown bullet list, so the 3+ item bullet-list rule doesn't apply here (consistent with how this was already treated in the prior review round).
  • No code fences, links, or cross-refs touched by this diff; the rest of the file (the all_of(names(x)) example, the x > 0 addition) is unchanged from the previously-reviewed commit and was already found correct.
  • No _site/, _freeze/, workflow, dependency, or bibliography changes.
  • No raw HTML, no new spell-check-risk terms.

This is a pure prose tightening with no new content, consistent with and fully resolving the only open note from the prior review (2026-10-12T09:40:21Z), whose verdict (Ready for merge) still stands and is now reinforced by this follow-up cleanup.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "958c5d33df3e6eea044518656065c62b1460da1f",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "chapters/coding-style/avoid-nesting.qmd lines 19-20 collapse two redundant sentences into one without changing meaning, removing the circularity noted in the prior review round.",
  "holistic_assessment": "Scope stays limited to the single prose file, no workflow or dependency or bibliography or build-artifact changes, and the fix matches the suggestion from the previous validated review."
}

Reviewed commit: a49c0e9

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $0.1769 (review) — run

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.

2026-10-12 21:38 UTC

@dem-extra1
dem-extra1 merged commit 48823b6 into main Oct 12, 2026
16 of 17 checks passed
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