Skip to content

Restore null-conditional operator for logger in WorkspaceOperations - #61

Closed
rbuergi with Copilot wants to merge 2 commits into
importfrom
copilot/sub-pr-59-again
Closed

rbuergi with Copilot wants to merge 2 commits into
importfrom
copilot/sub-pr-59-again

Conversation

Copilot AI commented Nov 1, 2025 •

Copy link
Copy Markdown
Contributor

Addresses review feedback about removing the null-conditional operator from logger usage in WorkspaceOperations.cs line 205.

Changes

  • Restored logger?.LogInformation(...) from logger.LogInformation(...) for defensive null safety

While the logger is obtained via GetRequiredService<ILoggerFactory>() which should guarantee non-null, the null-conditional operator prevents runtime exceptions if service registration is misconfigured and aligns with patterns used elsewhere in the codebase (e.g., TestBase, ImportManager).

// Before
logger.LogInformation("Applying changes to Data Stream {Stream}", stream.StreamIdentity);

// After
logger?.LogInformation("Applying changes to Data Stream {Stream}", stream.StreamIdentity);

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: rbuergi <6334612+rbuergi@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on import changes from review discussion Restore null-conditional operator for logger in WorkspaceOperations Nov 1, 2025
Copilot AI requested a review from rbuergi November 1, 2025 00:24
@rbuergi
rbuergi marked this pull request as ready for review November 1, 2025 00:24
@rbuergi rbuergi closed this Nov 1, 2025
@rbuergi
rbuergi deleted the copilot/sub-pr-59-again branch November 3, 2025 21:51
rbuergi added a commit that referenced this pull request Apr 23, 2026
…ityLog feed)

KernelContainer no longer exposes `IProgress<string> Progress` to scripts.
Scripts now resolve a standard `ILogger` via the `Log` global and call
`Log.LogInformation(...)` / `LogWarning(...)` / `LogError(...)` — messages
land on the run's ActivityLog MeshNode, which callers stream via
`GetRemoteStream<MeshNode, MeshNodeReference>` (same shape as Thread streams).

- KernelProgressReporter class + the hidden "progress" area stream: deleted.
- Default script usings gain `Microsoft.Extensions.Logging` so LogInformation
  etc. resolve without an extra using.
- OrleansKernelProgressTest rewritten to target the ActivityLog stream; tests
  are Skip-marked pending the ActivityLog-at-dispatch wiring in task #60.
- Doc/AI/ExecuteScript.md: "Watching Progress" section rewritten around the
  response's ActivityLog path and standard logger calls.

Note: `ProgressControl` (the UI progress bar in MeshWeaver.Layout) is
unchanged — different concept, nothing to do with the removed kernel global.

Follow-up (#60 / #61): create the ActivityLog MeshNode at dispatch, thread
the right ILogger into the script so Log calls actually reach that node.
Until then, Log messages go to the standard logger infrastructure and the
Skip-marked tests stay Skip.

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

2 participants