From becfb27e4666620108332965b74857c214e11252 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:28:23 -0500 Subject: [PATCH 1/2] Record settled design decisions for contributors Several settled decisions are written down nowhere in the repository, so a contributor or an AI assistant working here has to rediscover them or reopens them. - Add a Design decisions section to AGENTS.md: who approves public changes, services staying in this repository, what adding a service takes, why generic settings resolve in the core, and the Mg prefix convention. Record why boolean settings have no typed parameters. - Add three changelog conventions: placeholders that never worked, input the documented format never supported, and result values that depend on the service's response. - Add a section on the wiki's conventions. - Explain in RELEASING.md why develop does not allow GitHub's rebase merge, and how to get a clean history instead. - Explain at enum MessagingService why it is an enum rather than a string parameter with an argument completer. Contributor documentation only, so there is no changelog entry. --- AGENTS.md | 53 +++++++++++++++++++++++++++++++- RELEASING.md | 4 +++ ScriptMessage/ScriptMessage.psm1 | 4 +++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index cb036c6..68d8158 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,6 +72,9 @@ fix to something that itself landed after the last tag is invisible to users. Th never also appear as a "Fixed" entry, and it should not be listed among the functions a fix "affects." The same goes for a new configuration setting or message type. +**A placeholder that never worked is described by what it does now**, with no history and no "New Parameter" +label. In 1.1.1, `-MailType` was such a placeholder: its help read "TODO". + **A change the module adapted to is not a fix.** "Fixed" claims a defect in this module. If the code was never wrong and something underneath it changed (the Microsoft Graph API, the Microsoft Graph PowerShell SDK, or PowerShell itself), the entry belongs in **Changed**, or in **Added** when it gives callers something new. The @@ -108,6 +111,10 @@ Leave out: contributors, so link them from the commit message or the code, not from a user-facing entry. - **Anything with no observable effect on a caller.** If no script a user could reasonably write would have hit the bug, there is nothing to log. +- **Input the documented format never supported**, such as a true/false setting written as quoted text. It gets + no entry of its own, at most a general `Minor:` one. +- **Result values that depend on the service's response**, such as `Status` being `$true`. Help text leaves them + out too. Two or three sentences is a normal entry. Length is not thoroughness: someone scanning a release to see whether it affects them should get the answer in the first line. @@ -200,6 +207,33 @@ $Text = [System.IO.File]::ReadAllText($Path) [System.IO.File]::WriteAllText($Path, ($Text -replace "`r`n","`n")) # writes UTF-8 without a BOM ``` +## Design decisions + +These are settled. Don't reopen one without the maintainer. + +- **Exported functions' parameters and output change only with the maintainer's approval, and internal plumbing + never becomes a public parameter.** Pass plumbing to the private handlers instead. `Connect-ScriptMessage + -ServiceConfig` was added as plumbing and removed before it shipped for this reason. +- **Messaging services live in this repository, permanently.** Third-party drop-in service files are not a goal, + so no public handler contract gets designed or frozen. +- **Adding a service** takes one `MessagingService` enum value in `ScriptMessage/ScriptMessage.psm1`, one file in + `ScriptMessage/Services/`, and one `Register-ScriptMessageService` call at the bottom of that file. No function + in `ScriptMessage/Public/` switches on the service or calls a Microsoft Graph cmdlet. +- **`enum MessagingService` stays.** The comment above it in `ScriptMessage/ScriptMessage.psm1` says why; don't + replace it with a `[string]` parameter and an argument completer. +- **A generic setting is not necessarily supported by every service.** The module exists to standardize calls so + services are interchangeable, or so one call sends through several at once, which makes the unsupported case + normal rather than exceptional. A service that can't honor one reports it as the [Code](#code) section says. + There is no declaration of which settings each service supports. +- **Generic settings are resolved in the core, not in each service.** `MailType`, `ChatType`, and + `IncludeBCCInGroupChat` are shared vocabulary, so resolving them in each service would repeat the same merge + rule (a parameter beats the configuration file) in every one. A service resolves only its own prefixed + settings, such as `MgDisconnectWhenDone` and `MgApp_CertificatePath`. +- **A setting's name shows which layer owns it.** Every Microsoft Graph setting starts with `Mg` + (`MgPermissionType`, `MgDisconnectWhenDone`, `MgApp_*`), and generic settings have no prefix + (`AllowableMessageTypes`, `MailType`, `ChatType`, `IncludeBCCInGroupChat`). Give a new service's settings a + prefix of their own. + ## Code - **A new public function needs an explicit entry in `FunctionsToExport` in @@ -211,7 +245,9 @@ $Text = [System.IO.File]::ReadAllText($Path) - **A boolean configuration setting is an opt-in flag: on only when it equals `$true`.** Read it as `$ServiceConfig. -eq $true`, with the setting on the left, so missing, `false`, typos, and any quoted text except `"true"` (in any letter case, which counts as on) all mean off. Never read one with a `[bool]` cast, a truthiness test, or `$true -eq`: each of those reads the - text `"false"` as true. Name a new boolean setting so that `true` opts in and off is the safe state. + text `"false"` as true. Name a new boolean setting so that `true` opts in and off is the safe state. Typed + parameters per boolean setting were considered and rejected: configuration reaches a service as one object, so + they would need a restructure. - **A service that cannot honor a setting or parameter it receives says so in its result and sends what it can.** Add a `Warning` entry to that message's `Error` property, as the Microsoft Graph service does for a chat sent on behalf of someone else, rather than failing the whole call or dropping the value silently. @@ -250,3 +286,18 @@ one. `CHANGELOG.md` or other user-facing text. - **The safety rules above apply while gathering evidence:** no live sends without permission, and no tenant name or email domain in a note. + +## The wiki + +The wiki is the separate repository `Sekers/ScriptMessage.wiki`, and its only branch, `master`, publishes on +push. + +- **It describes the latest release**, with no version notes and nothing held back. Changes for the next release + are written ahead of it and pushed once that release has shipped. +- **A service's setup page is named after its `MessagingService` value**, with a hyphen between words so GitHub + shows a readable title: `Microsoft-Graph.md` for `MicrosoftGraph`. +- **Pages use LF line endings.** Put anything containing `$` in a code span or block, in case GitHub reads a pair + of dollar signs as math. +- **Its `.markdownlint.json` turns off MD041**, because GitHub shows each page's name as its title. Indent bullets + nested under bullets 2 spaces, and bullets under numbered steps 4. When re-indenting a bullet, move the tables + and paragraphs under it too; a linter quick fix moves only the bullet line. diff --git a/RELEASING.md b/RELEASING.md index a39ecaf..aa52caa 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -107,6 +107,10 @@ Rules for the shared branches: exist on `develop`, so every later release pull request would conflict with it. - **Never rebase or force-push `main` or `develop`.** Release tags cannot move, and rewriting shared history breaks every contributor's clone. Rebase only your own feature branch, and only before it merges. +- **`develop` does not allow GitHub's rebase merge**, because GitHub re-creates rebased commits without their + signatures ([About merge methods on GitHub](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github)). + For a clean history, rebase your feature branch locally onto `origin/develop` (Git re-signs each commit when + signing is set up), force-push that branch, and merge it with a merge commit. - **After every release or hotfix, merge `main` back into `develop` with a merge commit, never a squash or a rebase**, so `develop` contains the tagged release commit. A squash or a rebase copies `main`'s changes into new commits instead, so `develop` never contains the tagged commit, and a later release pull request can diff --git a/ScriptMessage/ScriptMessage.psm1 b/ScriptMessage/ScriptMessage.psm1 index 95657ee..697a21f 100644 --- a/ScriptMessage/ScriptMessage.psm1 +++ b/ScriptMessage/ScriptMessage.psm1 @@ -16,6 +16,10 @@ $script:ScriptMessageServiceTable = [ordered]@{} # Public Enum # Name: MessagingService +# An enum, not a [string] parameter with an [ArgumentCompleter]: the enum rejects an unknown service name when +# parameters bind, before anything runs, while a completer only suggests names. The generator form, +# [ValidateSet([IValidateSetValuesGenerator])], needs PowerShell 6 or later, so it is unavailable to a module +# that supports Windows PowerShell 5.1. enum MessagingService { MicrosoftGraph } From 4b4c5c27e35502d6bb5f34b23ccc613343b8edf4 Mon Sep 17 00:00:00 2001 From: Sekers <46898253+Sekers@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:45:40 -0500 Subject: [PATCH 2/2] Let the maintainer commit straight to develop when CI adds little Every change has to reach develop through a pull request, even a documentation edit whose tests already passed locally, so the maintainer waits on a test run before merging that adds little. Keep pull requests required for main and for contributors. Let the maintainer's own changes that a test run before merging wouldn't benefit go straight to develop after a local test run; the Tests workflow still runs on the push. Keep pull requests for squashing a branch. Contributor documentation only, so there is no changelog entry. --- RELEASING.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/RELEASING.md b/RELEASING.md index aa52caa..7bccf24 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -102,7 +102,11 @@ Gitflow without release branches, because only one version is maintained at a ti Rules for the shared branches: -- **Changes reach `develop` and `main` only through pull requests**, and the tests must pass first. +- **Changes reach `main` only through pull requests**, and the tests must pass first. +- **Changes reach `develop` through pull requests too, except the maintainer's own changes that a test run before + merging wouldn't benefit**, such as documentation. Those may be committed straight to `develop` after running + the tests locally, and the Tests workflow still runs on the push. Use a pull request anyway to squash a branch's + commits. - **Merge into `main` with a merge commit, never a squash or a rebase.** A squashed commit on `main` does not exist on `develop`, so every later release pull request would conflict with it. - **Never rebase or force-push `main` or `develop`.** Release tags cannot move, and rewriting shared history