elsa-workflows / elsa-workflows/elsa-foundation
[Cleanup] Batched low-severity nits: naming, dead code, and minor doc gaps across the codebase
- Dominant language
- C#
- Stars
- 5
- Forks
- 1
- Avg merge
- 3h 52m
- Merged PRs (30d)
- 212
Description
**Severity: low | Ease: easy** — batched trivial findings from the automated codebase quality audit, grouped by area. None of these are individually worth a dedicated issue; fix opportunistically or in one sweep.
**Primitives** (`src/Elsa/Primitives/**`): misspelled file `PropertyAccessorExtentions.cs` (should be `Extensions`); `StringExtensions.Camelize`/`Pascalize` use culture-sensitive `ToLower`/`ToUpper` instead of invariant; `ObjectExtensions.ConvertIEnumerableToArray` returns `null!` from a non-nullable signature; `TypeExtensions.GetMethods`'s XML doc promises validation the implementation doesn't perform; the `2020-01-01` identity epoch constant is duplicated across `ShortIdentityGenerator`/`SnowflakeIdentityGeneratorOptions`; `AddAsInterfaces` has no filter to exclude incidental interfaces like `IDisposable`; `SemVer`'s prerelease zero-padding width can produce inconsistent sort order for large numeric prerelease ids.
**Runtime** (`src/Elsa/Activities/Runtime/**`, `src/Elsa/Workflows/Runtime/**`): `ActivityBase.cs` carries ~35 lines of commented-out properties and first-person doubt-comments in the base class every activity extends; several constructors accept an unused `CancellationToken`; `ConstructActivityRequestHandler`/`BehaviorCollectionExtensions` do uncached reflection per call; `WorkflowExecutableNotFoundException` validates its argument after already building the exception message; inconsistent not-found exception types (`InvalidOperationException` vs `ArgumentException`) for equivalent failures; `RuntimeXxxCommandPayload` classes split between two different validation-exception strategies.
**Design/Api + ControlFlow** (`src/Elsa/Activities/Design/**`, `src/Elsa/Workflows/Design/**`, `src/Elsa/Activities/ControlFlow/**`): four empty placeholder endpoint classes (`Delete`/`Update`) with no explanation; `ListVersions.cs` constructed with `ILogger` instead of `ILogger` (copy-paste); `GetActivityExecutionEndpoint` is `public` and named inconsistently with its sealed/internal siblings; `Sequence.cs` has no XML doc unlike every sibling composite; `ArgumentDefinition`/`InputDefinition`/`OutputDefinition` have already drifted structurally with no test enforcing parity.
**Elsa3/Apps** (`src/Elsa3/**`, `src/Apps/Elsa.Server/**`): unused `JsonSerializerOptionsCache.cs`; duplicate private `ReadString` helper defined twice in the same file (`ElsaModuleManagementApi.cs`); `MapInput`/`MapOutput` copy-pasted apart from return type.
**Serialization/Secrets** (`src/Elsa/Serialization/**`, `src/Elsa/Secrets/**`): `TypeJsonConverter.cs:43` uses `reader.GetString()!` on a possibly-null token, causing `NullReferenceException` instead of a clear `JsonException`; `ObjectConverter.cs`'s `_defaultSerializerOptions` field is only used in its own pointless `??=`; `DefaultSecretResolver` has no `ILogger` and swallows exceptions to `e.Message` only; `Secret.LatestActiveVersion` calls `IsExpired()` with the real clock instead of the injected `TimeProvider`, disagreeing with `SecretLifecyclePolicy`'s clock.
**Proposed fix:** Sweep and fix each item above; none require design discussion.
_Found via automated codebase quality audit._
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by separating the listed findings by area, then inspect the named paths under src/Elsa/Primitives, Runtime, Design, ControlFlow, Elsa3/Apps, Serialization, and Secrets. Compare the referenced implementations and sibling types, and consider each item done when its stated inconsistency, dead code, naming issue, validation behavior, or documentation gap is addressed without leaving the remaining audit findings unresolved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- api, backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100