Skill to review whole PR#11031
Conversation
|
You've modified Claude Code configuration files. These files contain the shared configuration for all contributors. Please revert this change if unintended. ONLY modify these files if you need to change shared configurations that apply to everyone and have discussed this change with the TW team. To override the Claude settings locally, use For example, if you're changing AWS_PROFILE: Create or edit {
"env": {
"AWS_PROFILE": "your-sandbox-name"
}
} |
|
You've modified Claude Code configuration files. These files contain the shared configuration for all contributors. Please revert this change if unintended. ONLY modify these files if you need to change shared configurations that apply to everyone and have discussed this change with the TW team. To override the Claude settings locally, use For example, if you're changing AWS_PROFILE: Create or edit {
"env": {
"AWS_PROFILE": "your-sandbox-name"
}
} |
|
You've modified Claude Code configuration files. These files contain the shared configuration for all contributors. Please revert this change if unintended. ONLY modify these files if you need to change shared configurations that apply to everyone and have discussed this change with the TW team. To override the Claude settings locally, use For example, if you're changing AWS_PROFILE: Create or edit {
"env": {
"AWS_PROFILE": "your-sandbox-name"
}
} |
Changes suggested by docs-pr-review
|
You've modified Claude Code configuration files. These files contain the shared configuration for all contributors. Please revert this change if unintended. ONLY modify these files if you need to change shared configurations that apply to everyone and have discussed this change with the TW team. To override the Claude settings locally, use For example, if you're changing AWS_PROFILE: Create or edit {
"env": {
"AWS_PROFILE": "your-sandbox-name"
}
} |
|
|
||
| Do not worry about possible invalid internal links to anchors in the repo as these will be picked up by a separate testing tool after the site has been built. | ||
|
|
||
| Make no edits. |
There was a problem hiding this comment.
What do you think about changing this line in both review skills to something like "Make no edits. After the review, offer to fix straightforward issues like typos and broken links."
There was a problem hiding this comment.
I wonder if that gets too complex. My idea is that after being told what the issues are you can then say "Please fix the typo for "accommodation"?
And I don't want it to try to fix broken links. I'd rather have htmltest doing that - so far it has complained about links which are perfectly valid so I'd rather it didn't waste time (and tokens) trying to check them all. which is the reason behind line 14.
There was a problem hiding this comment.
Yes, definitely! You can still choose to ask it to apply certain changes afterwards if that feels like a more useful workflow.
|
|
||
| Follow links to other documents in the repo to check for inconsistencies if they seem to provide important related information. | ||
|
|
||
| Do not worry about possible invalid internal links to anchors in the repo as these will be picked up by a separate testing tool after the site has been built. |
There was a problem hiding this comment.
We test all internal links, right? Page-level links as well as anchors?
There was a problem hiding this comment.
Yes, we check any link which doesn't have an https:// in front of it. So internal to a page and from one page to another docs page.
There was a problem hiding this comment.
In my tests, Claude interpreted these instructions as "don't check internal references with # in them, but do check page-level internal references."
|
|
||
| Analyze all the changes in the current pull request or branch and return a list of suggestions or questions about any points to clarify, potential inconsistencies, and sections to restructure, add, or remove. Read the whole of each document modified in the pull request to ensure the changes do not make inconsistencies. | ||
|
|
||
| Follow links to other documents in the repo to check for inconsistencies if they seem to provide important related information. |
There was a problem hiding this comment.
I'm not sure how consistently Claude will bother reading other pages—it might be worth defining more specific triggers, like instructing it to always scan a couple related pages or even to look at every mentioned cross-reference (although "every" would probably need to have a max). It might also be worth specifying something along the lines of "inconsistencies, such as conflicting information, outdated references, or missing reciprocal links".
There was a problem hiding this comment.
I wanted to start with just a hint - and definitely not follow all links. But perhaps we can pick this up and do a more thorough look into what links it decides to follow and perhaps give something more programmatic.
My first idea is just to tell Claude that it can do this and that it doesn't have to stick just to the files in the PR.
MarkvanMents
left a comment
There was a problem hiding this comment.
Thanks for the review.
I've responded to your comments.
I'll re-run /docs-pr-review on the pr and see if it picks up anything from the rename or has any other suggestions.
|
|
||
| Analyze all the changes in the current pull request or branch and return a list of suggestions or questions about any points to clarify, potential inconsistencies, and sections to restructure, add, or remove. Read the whole of each document modified in the pull request to ensure the changes do not make inconsistencies. | ||
|
|
||
| Follow links to other documents in the repo to check for inconsistencies if they seem to provide important related information. |
There was a problem hiding this comment.
I wanted to start with just a hint - and definitely not follow all links. But perhaps we can pick this up and do a more thorough look into what links it decides to follow and perhaps give something more programmatic.
My first idea is just to tell Claude that it can do this and that it doesn't have to stick just to the files in the PR.
|
|
||
| Follow links to other documents in the repo to check for inconsistencies if they seem to provide important related information. | ||
|
|
||
| Do not worry about possible invalid internal links to anchors in the repo as these will be picked up by a separate testing tool after the site has been built. |
There was a problem hiding this comment.
Yes, we check any link which doesn't have an https:// in front of it. So internal to a page and from one page to another docs page.
|
|
||
| Do not worry about possible invalid internal links to anchors in the repo as these will be picked up by a separate testing tool after the site has been built. | ||
|
|
||
| Make no edits. |
There was a problem hiding this comment.
I wonder if that gets too complex. My idea is that after being told what the issues are you can then say "Please fix the typo for "accommodation"?
And I don't want it to try to fix broken links. I'd rather have htmltest doing that - so far it has complained about links which are perfectly valid so I'd rather it didn't waste time (and tokens) trying to check them all. which is the reason behind line 14.
|
You've modified Claude Code configuration files. These files contain the shared configuration for all contributors. Please revert this change if unintended. ONLY modify these files if you need to change shared configurations that apply to everyone and have discussed this change with the TW team. To override the Claude settings locally, use For example, if you're changing AWS_PROFILE: Create or edit {
"env": {
"AWS_PROFILE": "your-sandbox-name"
}
} |
|
Discussed with @dbreseman - merging |
No description provided.