-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added AreaPath and IterationPath migration with tests #85
base: master
Are you sure you want to change the base?
Conversation
Common/Config/ConfigJson.cs
Outdated
@@ -42,6 +42,14 @@ public class ConfigJson | |||
[DefaultValue(true)] | |||
public bool SkipExisting { get; set; } | |||
|
|||
[JsonProperty(PropertyName = "move-area-paths", DefaultValueHandling = DefaultValueHandling.Populate)] | |||
[DefaultValue(true)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd default this to false
Common/Config/ConfigJson.cs
Outdated
public bool MoveAreaPaths { get; set; } | ||
|
||
[JsonProperty(PropertyName = "move-iterations", DefaultValueHandling = DefaultValueHandling.Populate)] | ||
[DefaultValue(true)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for this one
/// <param name="workItemIds"></param> | ||
/// <param name="batchContext"></param> | ||
/// <param name="expand"></param> | ||
/// <returns></returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mind updating the comment block?
|
||
public bool IsEnabled(ConfigJson config) | ||
{ | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mind having this check config instead of just returning true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then you don't need to check later in process
} | ||
#endregion | ||
|
||
if (modificationCount > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not a fan of the pattern of passing an int and then assigning it in the return. Either make it an out or just have two variables.
{ | ||
Logger.LogInformation(LogDestination.All, $"Identified {context.SourceAreaAndIterationTree.AreaPathListLookup.Count} area paths in source project."); | ||
|
||
foreach (var ap in context.SourceAreaAndIterationTree.AreaPathList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's longer, mind spelling out areaPath?
|
||
foreach (var ap in context.SourceAreaAndIterationTree.AreaPathList) | ||
{ | ||
string areaPathInTarget = ap.Replace(context.Config.SourceConnection.Project, context.Config.TargetConnection.Project); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should already be a helper to replace project name. in this code above it's possible for a project name to be later on in the area path and you'll be replacing that too. Not sure if it's desired.
@@ -25,6 +26,7 @@ public abstract class BaseWorkItemsProcessor : IPhase1Processor | |||
|
|||
public abstract BaseWitBatchRequestGenerator GetWitBatchRequestGenerator(IMigrationContext context, IBatchMigrationContext batchContext); | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert?
using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; | ||
using Common.Config; | ||
using Logging; | ||
using Microsoft.TeamFoundation.TestManagement.WebApi; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what pulled this in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why I pulled in Microsoft.TeamFoundation.TestManagement.WebApi
. The others pulled in legitimately. I cleaned up the extras.
@@ -1,4 +1,5 @@ | |||
using System; | |||
using System.Linq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert?
Ran this utility, worked like a charm for my needs. Only difficulty I ran into was in trying to create iterations that had children. In our scenario we had a situation like this: Sprint 1 It came up with this error: Common.RetryPermanentException: Permanent error for 0aba7545-738c-49c7-8ce4-c69c774dea55, not retrying ---> Microsoft.VisualStudio.Services.Common.VssServiceException: VS402371: Classification node name Child Sprint 3 is already in use by a different child of parent classification node c55eef6f-c71b-4059-8a7e-4a656f29d4d2. Choose a different name and try again. I manually created this particular iteration relationship in the destination and it successfully went through. |
Added AreaPath and IterationPath migration with tests.