Conversation
Adds a Migrations section covering the dual Dapper/EF tracks, paths to the SSDT project and DbScripts, links to the contributing-docs MSSQL migration and SQL code style guides, and the local commands for applying migrations and regenerating EF migrations across all three providers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7689 +/- ##
==========================================
+ Coverage 60.20% 60.49% +0.29%
==========================================
Files 2137 2141 +4
Lines 94412 94654 +242
Branches 8409 8456 +47
==========================================
+ Hits 56840 57261 +421
+ Misses 35580 35388 -192
- Partials 1992 2005 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| - OpenTelemetry for observability (ADR 0020) | ||
| - Log to standard output (ADR 0021) | ||
|
|
||
| ## Migrations |
There was a problem hiding this comment.
❓Is this information Claude always needs, or only when dealing with schema changes? If the latter, can we convert this to a new Skill, or fold it into writing-database-queries?
There was a problem hiding this comment.
Only with db schemes. Not sure if the skill was triggered or not but if it was it is in-sufficient. I observed. the following defects.
- Hand wrote MySQL migration because the docker wasn't running. This should be forbidden.
- Used the wrong T-SQL syntax.
There was a problem hiding this comment.
Gotcha. Can we verify if it was invoked, and if the instructions need to be updated? If it wasn't invoked when you expected we should be able to adjust the skill's description or when_to_use fields to improve hits from natural language and inference.
There was a problem hiding this comment.
How do I do that? If you want to take ownership of bringing in this change that's fine too, I just don't want this happening again.
There was a problem hiding this comment.
I am of the opinon that the majority of the contributions below are important enough to add to the root CLAUDE.md. They are surgical and direct instructions for Claude.
I have a couple small recommendations.
There was a problem hiding this comment.
I am of the opinon that the majority of the contributions below are important enough to add to the root CLAUDE.md. They are surgical and direct instructions for Claude.
"Changing the DB schema" is only needed in specific circumstances, so putting it in CLAUDE.md directly contradicts the CONTRIBUTING.md guidelines that were just added to all of our repos.123
A lot of this section is already covered in the existing db related skills. Regardless of where it ultimately lands, it should not be duplicated in multiple places. Either move the existing skill content to CLAUDE.md or move the net-new content, here, to those skills.
If Claude is failing to invoke those Skills there are other approaches we can take to address that problem, like expanding the description, adding when_to_use, or adding a reference + usage table with instructions for using repo-specific skills here in CLAUDE.md. The latter option has value beyond sessions scoped to this repo, but is not in scope for this PR, so I recommend it as a separate follow-up to address that regardless of the decision of where to put this new information.
Footnotes
| We maintain an SSDT project at `src/Sql/dbo/` (current state of the database) and hand-written dated migrations | ||
| under `util/Migrator/DbScripts/`. Every schema change updates both. Apply locally with `pwsh dev/migrate.ps1`. | ||
|
|
||
| See: |
There was a problem hiding this comment.
I think that these are direct, deterministic instructions that we can codify in those rules files. Then, we can reference them here or in other places with little nudges/hints (@SaintPatrck can give you some tips @Hinton where we did that in the iOS and android repos to nudge Claude). These Contributing Docs rarely, if ever, change so we don't need to worry about documentation drift at this point IMO.
There was a problem hiding this comment.
It should prompt a fetch when it want's to act on it though?
There was a problem hiding this comment.
No, that's not going to happen the way that you assume it will happen. The file is a simple file based instruction file treated as plain text.
Claude code will never invoke a read of those URLs or any URLs embedded in our CLAUDE.md files. Any URLs should be used as reference materials for humans to improve a CLAUDE.md or similar file. Any information that Claude should leverage during a session needs to be inside .md files as plain text -or- you must create skills that leverage WebFetch or and MCP server to retrieve said data. The later is an expensive process which is why it's best practice to create supplemental example or reference files that Claude Code will progressively load as needed or directed by a human.
Co-authored-by: Mick Letofsky <mick.tosk@gmail.com>
|
3 participants
🎟️ Tracking
📔 Objective
Adds a Migrations section covering the dual Dapper/EF tracks, paths to the SSDT project and DbScripts, links to the contributing-docs MSSQL migration and SQL code style guides, and the local commands for applying migrations and regenerating EF migrations across all three providers.