mirror of
https://github.com/jokob-sk/NetAlertX.git
synced 2026-09-18 18:47:00 -04:00
DOCS+BE: skill updates, write_notifications fix #1793
This commit is contained in:
1 parent
3bf10f8378
commit
8dcd670d24
7 files changed
+22
-22
No files matched your search
@@ -15,6 +15,10 @@ Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything
|
||||
|
||||
Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
|
||||
|
||||
## Confirm before every push
|
||||
|
||||
Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
|
||||
|
||||
## Before any git command that changes shared state
|
||||
|
||||
Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
|
||||
@@ -7,15 +7,11 @@ description: How to analyze and respond to GitHub PR review comments in NetAlert
|
||||
|
||||
## Standing Rule: A Repeated Comment Becomes a Skill Update
|
||||
|
||||
jokob (the maintainer) has asked for this explicitly on real PRs, not just implied it:
|
||||
- PR #1739 (test-mocking/MAC-case feedback repeated across two PRs): "will those skill prevent the issues I had to comment on for you to fix in the last 2 PRs? ... will you next time follow these based on available guidelines and skills? if not update those to make this explicit"
|
||||
- PR #1744 (wrong `mylog()` level): "change 'warn' to 'none' and update skills so this doesn't happen again"
|
||||
If a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted; the fix is in *applying* it, not in the skill being incomplete.
|
||||
|
||||
So: if a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted (e.g. `logging-standards` already lists the valid `mylog()` levels; the PR #1744 fix was in *applying* it, not in the skill being incomplete).
|
||||
## Code Style
|
||||
|
||||
## Code Style (from real review history)
|
||||
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** Real feedback on a `max()`-based one-liner: "make it easier to read, using max() obfuscates logic." When a built-in/clever expression saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** When a built-in/clever expression (e.g. a `max()`-based one-liner) saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
|
||||
## Before Writing Any Test Code — Non-Negotiable Checklist
|
||||
|
||||
|
||||
@@ -15,6 +15,10 @@ Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything
|
||||
|
||||
Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
|
||||
|
||||
## Confirm before every push
|
||||
|
||||
Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
|
||||
|
||||
## Before any git command that changes shared state
|
||||
|
||||
Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
|
||||
@@ -7,15 +7,11 @@ description: How to analyze and respond to GitHub PR review comments in NetAlert
|
||||
|
||||
## Standing Rule: A Repeated Comment Becomes a Skill Update
|
||||
|
||||
jokob (the maintainer) has asked for this explicitly on real PRs, not just implied it:
|
||||
- PR #1739 (test-mocking/MAC-case feedback repeated across two PRs): "will those skill prevent the issues I had to comment on for you to fix in the last 2 PRs? ... will you next time follow these based on available guidelines and skills? if not update those to make this explicit"
|
||||
- PR #1744 (wrong `mylog()` level): "change 'warn' to 'none' and update skills so this doesn't happen again"
|
||||
If a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted; the fix is in *applying* it, not in the skill being incomplete.
|
||||
|
||||
So: if a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted (e.g. `logging-standards` already lists the valid `mylog()` levels; the PR #1744 fix was in *applying* it, not in the skill being incomplete).
|
||||
## Code Style
|
||||
|
||||
## Code Style (from real review history)
|
||||
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** Real feedback on a `max()`-based one-liner: "make it easier to read, using max() obfuscates logic." When a built-in/clever expression saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** When a built-in/clever expression (e.g. a `max()`-based one-liner) saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
|
||||
## Before Writing Any Test Code — Non-Negotiable Checklist
|
||||
|
||||
|
||||
@@ -15,6 +15,10 @@ Don't run `git checkout -b <new-branch>`, `git branch <new-branch>`, or anything
|
||||
|
||||
Absent other instructions, work lands on `next_release` directly — commit there, `git push` targets `origin next_release`. Don't invent a feature-branch/PR workflow unless asked for one.
|
||||
|
||||
## Confirm before every push
|
||||
|
||||
Ask for explicit confirmation immediately before running `git push`, even to the default `next_release` target. Don't fold a push into a larger task silently — surface it as its own step and wait for a go-ahead.
|
||||
|
||||
## Before any git command that changes shared state
|
||||
|
||||
Run `git status` and `git branch --show-current` first, and don't assume the branch you last left the repo on is still checked out — another process/terminal may have changed it.
|
||||
@@ -7,15 +7,11 @@ description: How to analyze and respond to GitHub PR review comments in NetAlert
|
||||
|
||||
## Standing Rule: A Repeated Comment Becomes a Skill Update
|
||||
|
||||
jokob (the maintainer) has asked for this explicitly on real PRs, not just implied it:
|
||||
- PR #1739 (test-mocking/MAC-case feedback repeated across two PRs): "will those skill prevent the issues I had to comment on for you to fix in the last 2 PRs? ... will you next time follow these based on available guidelines and skills? if not update those to make this explicit"
|
||||
- PR #1744 (wrong `mylog()` level): "change 'warn' to 'none' and update skills so this doesn't happen again"
|
||||
If a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted; the fix is in *applying* it, not in the skill being incomplete.
|
||||
|
||||
So: if a review comment corrects something a skill *should* already cover, don't just fix that one instance — update the relevant skill as part of addressing the comment, same PR, same turn. If no skill covers it yet, that's the signal to create one. Check the skill first, though — sometimes it already covers the point and just wasn't consulted (e.g. `logging-standards` already lists the valid `mylog()` levels; the PR #1744 fix was in *applying* it, not in the skill being incomplete).
|
||||
## Code Style
|
||||
|
||||
## Code Style (from real review history)
|
||||
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** Real feedback on a `max()`-based one-liner: "make it easier to read, using max() obfuscates logic." When a built-in/clever expression saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
- **Prefer explicit, readable logic over compact-but-opaque expressions.** When a built-in/clever expression (e.g. a `max()`-based one-liner) saves a line or two but a reader has to reverse-engineer *why* it produces the right answer, write it out as plain conditional logic instead.
|
||||
|
||||
## Before Writing Any Test Code — Non-Negotiable Checklist
|
||||
|
||||
|
||||
@@ -916,7 +916,7 @@ class CreateNotificationRequest(BaseModel):
|
||||
description="Notification content"
|
||||
)
|
||||
level: NOTIFICATION_LEVELS = Field(
|
||||
"info",
|
||||
"alert",
|
||||
description="Notification severity level"
|
||||
)
|
||||
|
||||
|
||||
Reference in new issue
Block a user