Implement ExportWorkItemMappingTool #3031
Conversation
WalkthroughAdds a new ExportWorkItemMappingTool (interface, implementation, options, validator), registers it in DI, wires it into CommonTools/TfsCommonTools, records source→target mappings during TFS work item processing, and persists mappings to a JSON file from the processor’s finally block. Changes
Sequence Diagram(s)sequenceDiagram
participant P as TfsWorkItemMigrationProcessor
participant C as CommonTools (ExportWorkItemMapping)
participant M as ExportWorkItemMappingTool
participant F as Target JSON File
loop per migrated work item
P->>C: ExportWorkItemMapping.AddMapping(sourceId, targetId)
C->>M: AddMapping(sourceId, targetId)
note right of M #D6EAF8: store in concurrent dictionary\nlog conflict if existing differs
end
P->>P: finally block
P->>C: ExportWorkItemMapping.SaveMappings()
C->>M: SaveMappings()
alt options.PreserveExisting == true
M->>F: Load existing mappings (if present)
note right of M #FDEBD0: merge, warn on conflicts
end
M->>F: Write indented JSON via atomic temp+move
note right of F #E8F8F5: persisted mapping file
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/MigrationTools/Tools/WorkItemMappingTool.cs (1)
24-24: Consider thread-safety for the mappings dictionary.
Dictionary<string, string>is not thread-safe. Whilst the current processor implementation processes work items sequentially, this creates a fragile dependency. If the processor is later refactored to process items in parallel, you'll have silent data races.Consider using
ConcurrentDictionary<string, string>instead to make this robust against future changes.- private readonly Dictionary<string, string> _mappings = []; + private readonly ConcurrentDictionary<string, string> _mappings = new();You'll also need to update
AddMappingto useTryAddorAddOrUpdate, and adjust the merge logic accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs(2 hunks)src/MigrationTools.Clients.TfsObjectModel/Tools/TfsStaticTools.cs(2 hunks)src/MigrationTools/ServiceCollectionExtensions.cs(1 hunks)src/MigrationTools/Tools/Interfaces/IWorkItemMappingTool.cs(1 hunks)src/MigrationTools/Tools/StaticTools.cs(2 hunks)src/MigrationTools/Tools/WorkItemMappingTool.cs(1 hunks)src/MigrationTools/Tools/WorkItemMappingToolOptions.cs(1 hunks)src/MigrationTools/Tools/WorkItemMappingToolOptionsValidator.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/MigrationTools/Tools/WorkItemMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
src/MigrationTools/Tools/Interfaces/IWorkItemMappingTool.cs (1)
src/MigrationTools/Tools/WorkItemMappingTool.cs (2)
AddMapping(36-56)SaveMappings(61-86)
src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs (3)
src/MigrationTools/Tools/StaticTools.cs (2)
CommonTools(8-48)CommonTools(37-47)src/MigrationTools/Tools/Interfaces/IWorkItemMappingTool.cs (2)
SaveMappings(18-18)AddMapping(13-13)src/MigrationTools/Tools/WorkItemMappingTool.cs (2)
SaveMappings(61-86)AddMapping(36-56)
src/MigrationTools/ServiceCollectionExtensions.cs (3)
src/MigrationTools/Tools/WorkItemMappingTool.cs (2)
WorkItemMappingTool(17-128)WorkItemMappingTool(26-33)src/MigrationTools/Tools/WorkItemMappingToolOptions.cs (2)
WorkItemMappingToolOptions(8-25)WorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/WorkItemMappingToolOptionsValidator.cs (1)
WorkItemMappingToolOptionsValidator(5-16)
src/MigrationTools/Tools/WorkItemMappingToolOptionsValidator.cs (1)
src/MigrationTools/Tools/WorkItemMappingToolOptions.cs (2)
WorkItemMappingToolOptions(8-25)WorkItemMappingToolOptions(10-13)
src/MigrationTools/Tools/WorkItemMappingTool.cs (3)
src/MigrationTools/Tools/WorkItemMappingToolOptions.cs (2)
WorkItemMappingToolOptions(8-25)WorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/Infrastructure/ITool.cs (1)
ITool(7-9)src/MigrationTools/Tools/Interfaces/IWorkItemMappingTool.cs (2)
AddMapping(13-13)SaveMappings(18-18)
🔇 Additional comments (5)
src/MigrationTools.Clients.TfsObjectModel/Tools/TfsStaticTools.cs (1)
25-25: LGTM!The wiring of the new IWorkItemMappingTool dependency follows the established pattern and integrates cleanly into the TfsCommonTools constructor.
Also applies to: 41-41, 44-44
src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs (2)
236-236: LGTM!Placing
SaveMappings()in the finally block ensures mappings are persisted even when exceptions occur during processing. This is the correct location for this call.
605-605: LGTM!The mapping is recorded at the right time: after the target work item is successfully created or identified, and the null check on line 603 ensures this is safe.
src/MigrationTools/ServiceCollectionExtensions.cs (1)
49-49: LGTM!The DI registration follows the established pattern and correctly wires the validator. Singleton scope is appropriate for a tool that accumulates mappings throughout the migration lifecycle.
src/MigrationTools/Tools/WorkItemMappingToolOptions.cs (1)
21-21: Fix the typo in the XML documentation."workt item mapping" should be "work item mapping".
- /// Gets the work item mapping tool for exporting workt item mapping. + /// Gets the work item mapping tool for exporting work item mapping.Likely an incorrect or invalid review comment.
satano
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptionsValidator.cs (1)
11-12: Fix const + string interpolation (won’t compile).const cannot be used with interpolated strings. Make it a local string.
Apply:
- const string msg = $"'{nameof(options.TargetFile)}' is not set, so work item mappings cannot be saved."; + string msg = $"'{nameof(options.TargetFile)}' is not set, so work item mappings cannot be saved.";src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
15-18: Fix typo in XML doc.“wher” → “where”.
- /// Path to file, wher work item mapping will be saved. + /// Path to file, where work item mapping will be saved.src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
74-85: Use atomic write, ensure directory exists, and serialise a snapshot.OpenWrite risks partial/tailed files and lost data on failure. Also, enumerating a live map while writing can race. Write to a temp file, then replace, and snapshot the map first.
Apply:
- Log.LogInformation("Saving work item mappings to file '{targetFile}'.", Options.TargetFile); - Dictionary<string, string> allMappings = _mappings; + Log.LogInformation("Saving work item mappings to file '{targetFile}'.", Options.TargetFile); + // Take an immutable snapshot to avoid races while serialising + Dictionary<string, string> allMappings = new(_mappings); if (Options.PreserveExisting) { Log.LogInformation($"'{nameof(Options.PreserveExisting)}' is set to 'true'." + " Loading existing work item mappings from '{targetFile}'.", Options.TargetFile); Dictionary<string, string> existingMappings = LoadExistingMappings(); allMappings = MergeWithExistingMappings(_mappings, existingMappings); } - using FileStream target = File.OpenWrite(Options.TargetFile); - JsonSerializer.Serialize(target, allMappings, _jsonOptions); + try + { + string? dir = Path.GetDirectoryName(Options.TargetFile); + if (!string.IsNullOrEmpty(dir)) + { + Directory.CreateDirectory(dir); + } + string tempFile = Options.TargetFile + ".tmp"; + using (FileStream fs = new FileStream(tempFile, FileMode.Create, FileAccess.Write, FileShare.None)) + { + JsonSerializer.Serialize(fs, allMappings, _jsonOptions); + } + // Replace atomically; keep a .bak in case + if (File.Exists(Options.TargetFile)) + { + File.Replace(tempFile, Options.TargetFile, Options.TargetFile + ".bak"); + } + else + { + File.Move(tempFile, Options.TargetFile); + } + } + catch (Exception ex) + { + Log.LogError(ex, "Failed to save work item mappings to '{targetFile}'.", Options.TargetFile); + throw; + }
94-96: Guard against null deserialisation result.JsonSerializer.Deserialize can return null; don’t propagate null.
Apply:
- using Stream source = File.OpenRead(Options.TargetFile); - return JsonSerializer.Deserialize<Dictionary<string, string>>(source); + using Stream source = File.OpenRead(Options.TargetFile); + return JsonSerializer.Deserialize<Dictionary<string, string>>(source) ?? new();
🧹 Nitpick comments (3)
src/MigrationTools/ServiceCollectionExtensions.cs (1)
23-28: Add ValidateOnStart for custom‑validator options to fail fast.The overload with a validator does not call ValidateOnStart, unlike the non‑validator overload (Line 20). Align behaviour to surface config errors at startup.
Apply:
- return services.AddOptions<TOptions>().Bind(configuration.GetSection(options.ConfigurationMetadata.PathToInstance)); + return services + .AddOptions<TOptions>() + .Bind(configuration.GetSection(options.ConfigurationMetadata.PathToInstance)) + .ValidateOnStart();src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs (1)
13-18: Consider returning a boolean from AddMapping.Returning bool (added vs. pre‑existing/conflict) improves call‑site logic and telemetry without extra lookups. Not required, but ergonomic.
Example:
- void AddMapping(string sourceId, string targetId); + /// <returns>True if a new mapping was added; false if it already existed or was ignored.</returns> + bool AddMapping(string sourceId, string targetId);I’ve provided a matching implementation change in the tool file comment.
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (1)
47-51: Severity level: consider Warning instead of Error on conflicting re-map.A conflicting re‑map may be expected in retries. Using LogWarning reduces noise while still surfacing the issue.
- Log.LogError("Attempt to map source work item ID '{sourceId}' to target ID '{targetId}'." + Log.LogWarning("Attempt to map source work item ID '{sourceId}' to target ID '{targetId}'."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs(2 hunks)src/MigrationTools.Clients.TfsObjectModel/Tools/TfsStaticTools.cs(2 hunks)src/MigrationTools/ServiceCollectionExtensions.cs(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingTool.cs(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingToolOptionsValidator.cs(1 hunks)src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs(1 hunks)src/MigrationTools/Tools/StaticTools.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs
- src/MigrationTools.Clients.TfsObjectModel/Tools/TfsStaticTools.cs
🧰 Additional context used
🧬 Code graph analysis (5)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
AddMapping(36-56)SaveMappings(61-86)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptionsValidator.cs (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
ExportWorkItemMappingToolOptions(8-25)ExportWorkItemMappingToolOptions(10-13)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
ExportWorkItemMappingToolOptions(8-25)ExportWorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs (2)
AddMapping(13-13)SaveMappings(18-18)
src/MigrationTools/ServiceCollectionExtensions.cs (3)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
ExportWorkItemMappingTool(17-128)ExportWorkItemMappingTool(26-33)src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
ExportWorkItemMappingToolOptions(8-25)ExportWorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/ExportWorkItemMappingToolOptionsValidator.cs (1)
ExportWorkItemMappingToolOptionsValidator(5-16)
🔇 Additional comments (2)
src/MigrationTools/Tools/StaticTools.cs (1)
20-24: LGTM on wiring and docs.Property and ctor parameter are correctly added and documented; DI will resolve given the registration in ServiceCollectionExtensions.
Also applies to: 35-46
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (1)
24-25: Collection expressions are fully supported—project explicitly uses LangVersion "default".The
MigrationTools.csprojinherits<LangVersion>default</LangVersion>fromDirectory.Build.props. This isn't a vague default—it's an explicit directive that forces the compiler's latest released major C# version, regardless of the target framework. LangVersion "default" (aka latestMajor) maps to the newest major C# version the compiler knows, forcing the compiler's latest major version regardless of TFM. The[]collection expression syntax is perfectly safe under this configuration. No refactoring needed.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (1)
106-121: Handle null return from JSON deserialisation.Past review flagged that
JsonSerializer.Deserializecan return null if the JSON is invalid or represents a null value. This will cause aNullReferenceExceptionwhen the returned dictionary is used inMergeWithExistingMappings.Apply this diff:
if (File.Exists(Options.TargetFile)) { using Stream source = File.OpenRead(Options.TargetFile); - return JsonSerializer.Deserialize<Dictionary<string, string>>(source); + return JsonSerializer.Deserialize<Dictionary<string, string>>(source) ?? []; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs
🧰 Additional context used
🧬 Code graph analysis (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
ExportWorkItemMappingToolOptions(8-25)ExportWorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs (2)
AddMapping(13-13)SaveMappings(18-18)
🔇 Additional comments (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (1)
123-145: LGTM: Merge logic correctly handles conflicts.The merge prioritises current mappings over existing ones when conflicts arise, with appropriate warning logging. The implementation is sound.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (1)
96-97: Still using Copy+Delete instead of atomic Move.This was flagged in past reviews:
File.Copyfollowed byFile.Deleteis not atomic and leaves the temp file if deletion fails. UseFile.Move(tempFile, Options.TargetFile, overwrite: true)for a single atomic operation.Apply this diff:
- File.Copy(tempFile, Options.TargetFile, overwrite: true); - File.Delete(tempFile); + File.Move(tempFile, Options.TargetFile, overwrite: true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (2)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
ExportWorkItemMappingToolOptions(8-25)ExportWorkItemMappingToolOptions(10-13)src/MigrationTools/Tools/Interfaces/IExportWorkItemMappingTool.cs (2)
AddMapping(13-13)SaveMappings(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build, Test, Sonar Cloud Analysis, & Package
🔇 Additional comments (3)
src/MigrationTools/Tools/ExportWorkItemMappingTool.cs (3)
20-26: Solid foundation: thread-safe and case-insensitive.The
ConcurrentDictionarywithStringComparer.OrdinalIgnoreCasecorrectly addresses the concurrency and case-sensitivity concerns from previous reviews. Clean.
37-56: AddMapping is now properly atomic.The
TryAddapproach correctly handles concurrent additions without TOCTOU races. The redundant assignment flagged in past reviews has been removed. Well done.
123-145: Merge logic is sound.The case-insensitive result dictionary and conflict resolution (current wins over existing, with warning) are both correct. The preference for current mappings is pragmatic.
|
@MrHinsh How can I make that the tool is disabled by default? In constructor of |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
10-13: Constructor setsEnabled = false, creating schema inconsistency.This contradicts the schema files which declare the default as
true. As noted in my review ofconfiguration.schema.json, you need to align these.Setting
Enabled = falseis actually the right choice for a tool that exports to disk—users should explicitly opt in. Update the schema files to match this code.docs/data/classes/reference.tools.exportworkitemmappingtool.yaml (1)
32-36: Documentation inherits theEnableddefault inconsistency.Line 35 states
defaultValue: true, but the actual code defaults tofalse. Once you fix the schema (as flagged inconfiguration.schema.json), update this to match.- parameterName: Enabled type: Boolean description: If set to `true` then the tool will run. Set to `false` and the processor will not run. - defaultValue: true + defaultValue: false isRequired: false dotNetType: System.Boolean, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798edocs/static/schema/schema.tools.exportworkitemmappingtool.json (1)
8-12: Two issues with theEnabledproperty schema.
- Default value inconsistency (duplicate): The default is
"true"but the code setsfalse. Fix this to match the code.- Type mismatch: The default is a string
"true"but the property type is boolean. This should be the boolean literaltrue(orfalseonce you fix issue #1)."Enabled": { "description": "If set to `true` then the tool will run. Set to `false` and the processor will not run.", "type": "boolean", - "default": "true" + "default": false },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/data/classes/reference.tools.exportworkitemmappingtool.yaml(1 hunks)docs/static/schema/configuration.schema.json(1 hunks)docs/static/schema/schema.tools.exportworkitemmappingtool.json(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
🔇 Additional comments (2)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
15-26: Property defaults are sensible.
TargetFile = string.Emptyis appropriate; the validator will enforce it's set when enabled.PreserveExisting = trueis the safer default for merging behaviour—avoids accidentally clobbering existing mappings.docs/data/classes/reference.tools.exportworkitemmappingtool.yaml (1)
52-53: Status comments are acceptable placeholders.The "missing XML code comments" notes are standard documentation placeholders. Address them when you add proper XML documentation to the tool class itself.
|
The XML tag controls the documentation, and the variable setting in the contractor controls the default. They should match. |
Well. So how can I make this tool disabled by default and the documentation to match it? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
13-13: Use lowercasestringin XML documentation.C# style conventions prefer the lowercase keyword alias over the BCL type name.
- /// <default>String.Empty</default> + /// <default>string.Empty</default>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
🔇 Additional comments (1)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
16-21: LGTM!The
PreserveExistingproperty is correctly implemented with clear documentation that matches the default value. The use of<see langword="true"/>in the summary is a nice touch.
satano
force-pushed
the
workItemMappingTool
branch
from
038d709 to
c8e00c5
Compare
|
OK. A realized, that even if |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/static/schema/configuration.schema.json (1)
2253-2257: Fix JSON Schema default value type.Line 2256 sets
default: "null"as a string literal. For an optional nullable string property, usedefault: null(JSON null) or omit the default field.Apply this diff:
"ChangeSetMappingFile": { "description": "Path to changeset mapping file.", "type": "string", - "default": "null" + "default": null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/data/classes/reference.tools.tfschangesetmappingtool.yaml(1 hunks)docs/static/schema/configuration.schema.json(2 hunks)docs/static/schema/schema.tools.tfschangesetmappingtool.json(1 hunks)src/MigrationTools.Clients.TfsObjectModel/Tools/TfsChangeSetMappingToolOptions.cs(1 hunks)src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
src/MigrationTools.Clients.TfsObjectModel/Tools/TfsChangeSetMappingToolOptions.cs (1)
src/MigrationTools/Tools/Infrastructure/ToolOptions.cs (1)
ToolOptions(9-31)
🔇 Additional comments (4)
src/MigrationTools/Tools/ExportWorkItemMappingToolOptions.cs (2)
8-21: Clean and pragmatic design.The options class is straightforward: two properties with sensible defaults and clear documentation.
PreserveExisting = trueis the right conservative choice—won't accidentally clobber existing mappings. No fuss, no ceremony, just what's needed.
14-14: Verification confirms validator enforcement is correct—no issues found.The validator at
src/MigrationTools/Tools/ExportWorkItemMappingToolOptionsValidator.cscorrectly enforces the TargetFile requirement. Line 9 checksif (options.Enabled && string.IsNullOrWhiteSpace(options.TargetFile))and fails validation with the message"'TargetFile' is not set, so work item mappings cannot be saved."The validator is properly registered in the DI container, and the tool implementation includes defensive checks as well. The empty string default is the right design choice—it's explicit about requiring configuration whilst the validator ensures it's enforced at runtime.src/MigrationTools.Clients.TfsObjectModel/Tools/TfsChangeSetMappingToolOptions.cs (1)
1-13: LGTM! Clean documentation update.The XML documentation accurately describes the property, and the default tag correctly reflects that an uninitialised string property defaults to null. Removing unused imports is good housekeeping.
docs/data/classes/reference.tools.tfschangesetmappingtool.yaml (1)
52-53: Documentation correctly reflects the source code.The description and defaultValue now match the XML documentation in TfsChangeSetMappingToolOptions.cs.
|
I changed this PR to draft. I will run this tool while migrating bigger project to test it and activate it after. |
2 participants
Implementation of
ExportWorkItemMappingToolThis tool is for saving mappings for work items between source and target. Work item IDs are saved as simple dictionary source ID → target ID.
We need such a mappings for some post processing after work item migrations. In our case, we use wiki in source system, which will also be migrated to target. In the wiki, there are many links to backlog items. These links are written in the wiki as simple
#{workItemId}– for example#1234567. So if we will have mappings from source to target, we can just replace those IDs and the wiki will be working in target system.I believe, some users can also find it useful for some post-processing.
Summary by CodeRabbit
New Features
Tests
Documentation