Move RIT to build against net472 and net6.

[JoeRobich]

This change would set us up to move to a dotnet-tool. I tried to target netstandard1.3 but our dependencies didn't want to cooperate.

Moving from net46 to net472 does necessitate updating insertion pipelines to invoke RIT from the correct folder. This has been completed for Roslyn and Roslyn-SDK. Would need to coordinate with the project-system & the testing teams to make sure no one gets broken.

[RikkiGibson]

Looks fine to me, however, wondering if we can drop the netframework target entirely. Does that make deployment harder than it currently is?

[JoeRobich]

Looks fine to me, however, wondering if we can drop the netframework target entirely. Does that make deployment harder than it currently is?

It is a bigger change, but not necessarily a hard change. Still need to check with @dmonroym and @shyamnamboodiripad that the current change will be low impact enough to get completed.

[shyamnamboodiripad]

Tagging @AbhitejJohn as well. @JoeRobich I don't think it would be too difficult to modify the pipelines for UT insertion as they are based on corresponding roslyn pipelines - but Abhitej should have more context on this.

We may also need to update the oneoffinsertion, updatexistingpr, overwriteexistinpr queues. Joey - will you be fixing those?

[JoeRobich]

We may also need to update the oneoffinsertion, updatexistingpr, overwriteexistinpr queues. Joey - will you be fixing those?

I've already updated those pipelines to handle this change. Basically I update the powershell script task in our pipelines to test for a net472 directory before falling back to the current net46.

See comment below

[RikkiGibson]

Should we attempt to do #854 before merging this change?

[RikkiGibson]

Basically, there is so much copy-pasting in web UIs to configure our pipelines. If we just had a YAML template for "obtain RIT" and "run RIT" tasks, this kind of maintenance could potentially get easier. I don't know for sure if AzDO supports our particular use case with this, though.

[dmonroym]

Seems like we just need to modify the script on the release pipeline? From the project-system side of things it seems low impact and I can test this out once it's ready for us to do so.

[JoeRobich]

Should we attempt to do #854 before merging this change?

I've updated all of Roslyn's insertion pipelines.

[JoeRobich]

Sigh, I goofed when I updated Roslyns insertion pipelines. I shouldn't have changed the working directory. Updated again with the intention of removing the if when this merges.

mv RoslynTools.VisualStudioInsertionTool.* RIT
if (Test-Path .\RIT\tools\net472) {
  .\RIT\tools\net472\OneOffInsertion.ps1 ...
}
else {
  .\RIT\tools\net46\OneOffInsertion.ps1 ...
}

[JoeRobich]

Thanks Remove Unused Reference feature!

[RikkiGibson]

Yeah whoever added that is great

[JoeRobich]

Thanks @AlekseyTs!

[RikkiGibson]

Hi Joey, how is it going with this PR? Please let me know if there's any way I can assist with it.

[JoeRobich]

Hi Joey, how is it going with this PR?

This PR should be ready to merge. I'm just hesitant to break everyone. =p

[dmonroym]

Hi Joey, how is it going with this PR?

This PR should be ready to merge. I'm just hesitant to break everyone. =p

@RikkiGibson perhaps we can publish the package with this update elsewhere and dogfood it for a bit before publishing it in dotnet-eng

[RikkiGibson]

Sure, it feels like we could publish to some testing feed and use the "OneOffInsertion - Experimental" pipeline to try out this change.

[RikkiGibson]

I think most people run the tool via the powershell scripts, so I'm optimistic things won't be broken too badly by this change. Maybe we can merge in a week or two?

[JoeRobich]

@RikkiGibson Think we should move forward with this? One thought is we could rework this tool with the System.CommandLine api and convert the ps1 scripts to be subcommands of the RIT tool.

[RikkiGibson]

Yeah, let's do this. It seems like this PR is a first step toward what you've described.

[JoeRobich]

Instead of taking this PR (#906) we should move RIT functionality to dotnet-roslyn (#1168) as a set of subcommands which correspond to various scripts create-insertion (OneOffInsertion), create-dummy-insertion (CreateDummyPR), update-insertion, & overwrite-insertion (UpdateExistingPR).

Closing this PR.

[RikkiGibson]

Should we just copy-paste the parts of the implementation we need into a new command under the dotnet-roslyn CLI tool here? I recall having problems trying to share code between this and a dotnet CLI tool project.

[JoeRobich]

We shouldn't try to share code. I would argue that we remove the old RIT implementation when the code is moved. Users of RIT won't be broken by the change but would then need to move to get new functionality.

[RikkiGibson]

I think I'm fine with removing. It shouldn't be hard for users to move from the old to new tool if they need some fix.

This might be a good opportunity for us to start using semantic versioning of the CLI tool, i.e. bumping the major version when we make breaking changes. Then we'll ensure that our users pull down the tool with the correct kind of version range (i.e. including patch/minor releases, not major releases).