diff-match-patch
diff-match-patch copied to clipboard
Put C# port under solution
The reason of this PR is moving all C# related files under solution, as a target I used .netstandard2.0
.
Benefits:
-
netstandard
as a target - simplified process of making changes in source code
- better approach for running tests (Unit + Performance(BenchmarkDotNet))
- **can be shipped as a
NuGet
package in future
Source code of core functionality I left as-is.
View of Solution explorer
:
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here (e.g. I signed it!
) and we'll verify it.
What to do if you already signed the CLA
Individual signers
- It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
Corporate signers
- Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
- The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
- The email used to register you as an authorized contributor must also be attached to your GitHub account.
I signed it!
CLAs look good, thanks!
Hi, thanks for this PR.
The main issue here is how to support the existing developers who depend on the DMP library. They have automated build scripts that pull DMP into their projects. Wouldn't this PR break all existing developers?
@rolshevsky if you're interested, I am maintaining a 'canonical C#' version of the original C# version at https://github.com/jhgbrt/google-diff-match-patch-csharp/.
I'd like to see this merged, if possible. Would the potential breakage for other developers be remedied if instead of moving the csharp file, it stayed where it was?
@mattkoch614 @NeilFraser , I think, for saving compatibility I can restore DiffMatchPatch.cs
file in original path, and in this case it will not break usage flow for existing developers. Then we can mark this file as deprecated
in header (comment or something) and continue development in solution project.
Is it make sense?
@rolshevsky
It is possible to add the sln and csproj files without breaking the folder structure:
- add a .csproj in the root project for the core library, excluding the tests folder in the xml:
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<DefaultItemExcludes>$(DefaultItemExcludes);tests\**</DefaultItemExcludes>
</PropertyGroup>
</Project>
- Add a separate csproj file for the tests in the corresponding tests folder in the root folder, add the sln file, and add both csproj files to that sln file
@jhgbrt 's suggestion sounds reasonable. My main reason for wanting this merged is to have an easy way of creating a *.nupkg
file so this library can be hosted on Nuget. See #36
EDIT: I added my own PR to resolve these issues. #47
@NeilFraser, @jhgbrt, @mattkoch614 - I was updated PR with the following changes:
- I restored
DiffMatchPatch.cs
in original folder by original path:csharp/DiffMatchPatch.cs
, so we have no any braking changes anymore for existing users. - I put this original
DiffMatchPatch.cs
out of solution. It will keep backward compatibility, and all further development supposed to be done from under solution. - I marked this file and all public classes inside as
Obsolete
. In future, when we will getNuGet
support we can add some more informative messages that will say that user can switch from this old version toNuGet
package.
@mattkoch614 I have some doubts about #47 where we included original DiffMatchPatch.cs
into solution, because as it was mentioned by @NeilFraser we need to keep backward compatibility for existing users. With current approach we keeping original file as-is outside the solution.
@rolshevsky Thanks for the information.
@rolshevsky @jhgbrt @NeilFraser
Correct me if I am wrong, but isn't the whole point to have the C# library added to a .NET project and solution while keeping file paths unchanged? How would this break compatibility for anyone? Any scripts that exist that pull down the DiffMatchPatch.cs
file would theoretically still function. All #47 does is wrap existing files (without moving them) into a project and solution with some added metadata for an eventual Nuget package.
Forgive me if I am missing something.
@mattkoch614
Any scripts that exist that pull down the DiffMatchPatch.cs file would theoretically still function.
Yeah, exactly, I mean if we left path as-is and then start patching the library API (#25) - this will be a breaking change for existing users. But in case if we just deprecate existing DiffMatchPatch.cs
we can keep making changes in project from under solution and just bumping NuGet
package version, and in this case it will not affect existing users.
But again, it's just my understanding of the problem here. @NeilFraser please, correct me if I'm wrong.
I would be pretty useful to have a more modern c# version of this. It seems to me that, if we want to assume full compatibility, the only way forward is to use the existing cs file as-is with a solution/project.
@Poltuu to some extends, it is implemented in that way already. I left original .cs
file as is to be fully compatible. Under solution I put a copy of the original .cs
file and for solution of course we can add any kind versioning we need (semver etc.), and then we can publish it on a NuGet as well.