Skip to content

feat: update documentation on field omission for updates and upserts#28

Merged
JanKulhavy merged 2 commits into
mainfrom
document-field-upsert
May 27, 2026
Merged

feat: update documentation on field omission for updates and upserts#28
JanKulhavy merged 2 commits into
mainfrom
document-field-upsert

Conversation

@JanKulhavy

@JanKulhavy JanKulhavy commented May 26, 2026

Copy link
Copy Markdown
Collaborator

This PR strengthens the Make skills documentation by explicitly warning that update/upsert/patch modules must omit mapper fields that should remain unchanged, because mapping an empty string can silently overwrite existing record data.

Copilot AI review requested due to automatic review settings May 26, 2026 15:24
@JanKulhavy JanKulhavy requested a review from a team as a code owner May 26, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the Make skills documentation by explicitly warning that update/upsert/patch modules must omit mapper fields that should remain unchanged, because mapping an empty string can silently overwrite existing record data.

Changes:

  • Added a new “Field Omission on Updates and Upserts” section with rationale and examples in make-module-configuring/mapping.md.
  • Introduced a new cardinal rule in make-module-configuring/SKILL.md and reinforced the guidance in general-principles.md.
  • Added a cross-skill “gotcha” note in make-scenario-building/mapping.md pointing readers to the new section.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
skills/make-scenario-building/mapping.md Adds a gotcha warning about empty-string mappings on update/upsert/patch modules and links to the configuring skill for details.
skills/make-module-configuring/SKILL.md Adds a new cardinal rule requiring omission of unwritten mapper fields on updates/upserts/patches.
skills/make-module-configuring/mapping.md Adds a dedicated section explaining why mapper field omission matters, with wrong/right examples and guidance for clearing fields.
skills/make-module-configuring/general-principles.md Extends the “empty vs null vs missing” gotcha to explicitly call out update/upsert mapper overwrite risk and links to the new section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/make-module-configuring/mapping.md Outdated
Comment thread skills/make-scenario-building/mapping.md Outdated
- mapping.md:115 (r3304900933): reword "function" → "keyword" for IML
  `erase`; matches the Keywords table in iml-expressions.md (alongside
  `null` and `ignore`) and avoids implying invocation as `erase(...)`.
- scenario-building/mapping.md:85 (r3304901017): clarify that the empty
  string is the *value*, not the key — "including a key with `""`" was
  ambiguous; reworded to "including a field with an empty-string value
  (`""`)".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@JanKulhavy JanKulhavy merged commit 91ce553 into main May 27, 2026
2 checks passed
@JanKulhavy JanKulhavy deleted the document-field-upsert branch May 27, 2026 10:47
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.

3 participants