GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add deployment environments to events by ejsmith · Pull Request #193 · exceptionless/Exceptionless.JavaScript Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Address the two environment-normalization issues and stale documentation examples before approval.
Adds deployment-environment support across the JavaScript SDK, including configuration defaults, per-event overrides, normalization, and environment-aware duplicate detection.
Changes:
configuration.environment and event-level environment APIs.| File | Summary | Review notes |
|---|---|---|
README.md |
Documents environment configuration. | Nit (1 vote): update remaining defaultData examples. |
packages/core/test/plugins/default/DuplicateCheckerPlugin.test.ts |
Tests environment-aware deduplication. | — |
packages/core/test/plugins/default/ConfigurationDefaultsPlugin.test.ts |
Tests defaults and overrides. | — |
packages/core/test/configuration/Configuration.test.ts |
Tests configuration normalization. | — |
packages/core/src/Utils.ts |
Normalizes environment values. | Moderate (1 vote): validate control characters before trimming and lowercase before length validation. |
packages/core/src/plugins/default/DuplicateCheckerPlugin.ts |
Separates duplicate tracking by environment. | — |
packages/core/src/plugins/default/ConfigurationDefaultsPlugin.ts |
Applies environment defaults. | — |
packages/core/src/models/Event.ts |
Adds the event environment field. | — |
packages/core/src/EventBuilder.ts |
Adds per-event environment overrides. | — |
packages/core/src/configuration/Configuration.ts |
Adds normalized environment configuration. | — |
.agents/skills/exceptionless-javascript/SKILL.md |
Updates SDK guidance. | Nit (1 vote): update stale legacy environment examples. |
.agents/skills/exceptionless-javascript/references/sending-events.md |
Documents event overrides. | — |
.agents/skills/exceptionless-javascript/references/configuration.md |
Documents configuration behavior. | Nit (1 vote): update stale deployment examples. |
.agents/skills/exceptionless-javascript/SKILL.md:12
references/client-core.md still has a copyable setup example that writes the deployment environment to config.defaultData["deployment"]. That guidance will not populate the new top-level field or its environment-aware duplicate behavior; update the stale reference (and other package examples using the legacy shape) in this change.Deployment environments use `config.environment` or `config.setEnvironment(name)` as the default, and `builder.setEnvironment(name)` for overrides. They serialize as top-level `environment`, separately from `data.@environment`. See [configuration.md](references/configuration.md).
.agents/skills/exceptionless-javascript/references/configuration.md:34
config.environment as the deployment-environment API, but other copyable SDK guidance still uses defaultData["deployment"] (references/client-core.md:25), a DeploymentPlugin that writes data.deployment (references/plugins.md:44), and the React Native README's defaultData["environment"] example (packages/react-native/README.md:82). Those examples bypass the new top-level field, so update the stale deployment examples in the same documentation change.Set `config.environment = "production"` or call `config.setEnvironment("production")`. Per-event `setEnvironment("staging")` overrides the default. Names are trimmed and lowercased; empty names, names longer than 64 characters, and control characters are ignored. Missing values remain unspecified. This property is independent of `data.@environment` runtime metadata and of the application version. It does not change server stack grouping or create per-environment status.
README.md:11
defaultData (for example, .agents/skills/exceptionless-javascript/references/client-core.md:25 and packages/react-native/README.md:82). Those snippets emit nested event data rather than the new top-level environment, so please update the remaining setup examples in the same change.Set a deployment environment in startup configuration with `config.environment = "production"` (or `config.setEnvironment("production")`). Override it on an event with `Exceptionless.createLog("Example").setEnvironment("staging").submit()`. Names are trimmed, lowercased, and limited to 64 characters. Missing or invalid names remain unspecified. The top-level `environment` is separate from machine/runtime diagnostics in `data.@environment`; stacks and fixed versions remain shared across environments.
packages/core/src/Utils.ts:568
trim() removes leading/trailing tabs and newlines before the regex runs, so values such as "\nproduction\n" are accepted as production. Validate the original input for control characters before trimming so all control-containing names remain unspecified. const name = value.trim();
// eslint-disable-next-line no-control-regex -- Deployment names cannot contain control characters.
return name && name.length <= 64 && !/[\u0000-\u001f\u007f-\u009f]/u.test(name) ? name.toLowerCase() : undefined;
packages/core/src/Utils.ts:568
"\u0130".repeat(64) becomes 128 code units), so this can emit an environment longer than the documented/server 64-character limit. Lowercase first and validate the normalized value. return name && name.length <= 64 && !/[\u0000-\u001f\u007f-\u009f]/u.test(name) ? name.toLowerCase() : undefined;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: af5bea4acc
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 638593309e
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dedcec3a8
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ents # Conflicts: # .agents/skills/exceptionless-javascript/SKILL.md # packages/core/src/plugins/default/DuplicateCheckerPlugin.ts
Document that an invalid per-event name does not fall back to the default, move the README guidance into the configuration section, test clearing an override, trimming control whitespace, and omitting an unset environment, and apply the lint rules from main. Co-Authored-By: Claude Opus 5.5 <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 55b2fa95f8
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
A plugin that runs after the defaults plugin could set an untrimmed, oversized, or control-character environment that reached the queue. Normalize again before submitting and before resubmitting merged duplicates. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Names are trimmed, then must match ^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$.
Anything else is unspecified and never rewritten.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
At least 1 approving review is required to merge this pull request.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.