YamlDotNet icon indicating copy to clipboard operation
YamlDotNet copied to clipboard

Unity fixes for 16.1.3

Open steinbitglis opened this issue 1 year ago • 2 comments

Welcome!

Thanks for your interest in contributing to this project. Any contribution will be gladly accepted, provided that they are generally useful and follow the conventions of the project.

  1. Please create one pull request for each feature. This results in smaller pull requests that are easier to review and validate.

  2. Avoid reformatting existing code unless you are making other changes to it.

    • Cleaning-up of usings is acceptable, if you made other changes to that file.
    • If you believe that some code is badly formatted and needs fixing, isolate that change in a separate pull request.
  3. Always add one or more unit tests that prove that the feature / fix you are submitting is working correctly.

  4. Please describe the motivation behind the pull request. Explain what was the problem / requirement. Unless the implementation is self-explanatory, also describe the solution.

    • Of course, there's no need to be too verbose. Usually one or two lines will be enough.
  5. Follow the project's coding conventions

steinbitglis avatar Oct 22 '24 07:10 steinbitglis

Yes okay I believe these are not "fixes" but a syntax downgrade specific to your environment. Maybe a fork is more suited for you? If you look at the commit history of this project you'll see all that changes you're reverting, were intentional.

I work with Unity too so we made a local fork of that project that undid those syntax changes, and we use it as a submodule integrated in the project. You should probably do something similar if that's what you need, but upstream is unlikely to accept your downgrade.

You can also use a pre-built DLL targeting netstandard2.0 in your Unity project, if you don't need to debug the source, in which case the syntax used to build the DLL is irrelevant

Rackover avatar Jan 24 '25 08:01 Rackover

This is the fork that I'm maintaining: https://assetstore.unity.com/packages/tools/integration/yamldotnet-for-unity-36292 I don't love doing it, but it makes the lives of Unity users easier. And I want them to have somewhat recent versions of YamlDotNet, as they have had that every time I've updated it.

Since the downgrade does not significantly degrade the source code, I think that the changes should be accepted. There is (or used to be) some code in the repository specific for Unity, just so that it would be compatible. That's why I thought my contributions would be welcome. If you want to be incompatible with Unity for some small amounts of syntactic sugar (https://en.wikipedia.org/wiki/Syntactic_sugar), then be my guest. I won't maintain the fork anymore then.

  • Fredrik

On Friday, January 24th, 2025 at 09:39, Louve @.***> wrote:

Yes okay I believe these are not "fixes" but a syntax downgrade specific to your environment. Maybe a fork is more suited for you? If you look at the commit history of this project you'll see all that changes you're reverting, were intentional.

I work with Unity too so we made a local fork of that project that undid those syntax changes, and we use it as a submodule integrated in the project. You should probably do something similar if that's what you need, but upstream is unlikely to accept your downgrade.

You can also use a pre-built DLL targeting netstandard2.0 in your Unity project, if you don't need to debug the source, in which case the syntax used to build the DLL is irrelevant

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

steinbitglis avatar Jan 24 '25 09:01 steinbitglis