Replies: 6 comments 4 replies
-
I am recompiling QuikGraph on .net 5 and netstandard 2.1 and netcore 3.1 (still multitargeting as the current code) but found some unit tests dont pass, others werent running correctly on the downloaded source on the older versions. |
Beta Was this translation helpful? Give feedback.
-
Hi,
Many thanks for the follow up.
I made my changes locally, and although a couple of things don't work in
.net 5 I may be able to put a formal fork on my repo so you can be able to
see what I changed.
And then we can check what is worth moving into your branch.
I may be able to do this on next week, but may need to have some guide as
how to create my repo forking from yours, because now I made it the "dummy"
way, I cloned from your repo and then I uploaded it into my repository,
After that I did my changes directly on top of that, i.e. I worked
directly.on the same branch. As you can see, I am still struggling with git
I'd be happy if you check what I have in my current repos:
https://github.com/VirtualZen/QuikGraph (should be your code)
https://github.com/VirtualZen/QuikGraph5 (changes for .net 5, As I was in a
hurry, didnt change all of your sln files as I saw that there is a nuget
mode and a project mode but I only made changes on one of your solutions.)
Regards
Paul
…On Wed, Jul 7, 2021 at 3:37 PM Alexandre Rabérin ***@***.***> wrote:
Any news on your side @VirtualZen <https://github.com/VirtualZen>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#25 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKWMYBAKDY3GYQKUAZECW2DTWS3JJANCNFSM4UZZOJ7A>
.
|
Beta Was this translation helpful? Give feedback.
-
Whatever you do, please keep around .NET Standard 2.0 compatibility; Unity still uses it. It's about to add support for .NET Standard 2.1, but not all versions will necessarily use it. |
Beta Was this translation helpful? Give feedback.
-
Getting back to initial question about target here are some details of what I think about each of them:
.NET 5 target issues are mainly related to the Serializable aspect of things. And by this I mean the binary serialization. which is using this attribute. Impacted libraries are mainly QuikGraph.Serialization and QuikGraph.Graphviz. The main point is the BinaryFormatter that become Obsolete due to severe security issues. So basically there is a need to replace this really old way of doing binary serialization. For that if you have any suggestion feel free to propose them. On the long run we for sure can think about adding other kinds of serialization like Json, in addition to what is currenlty available, but that's another topic. |
Beta Was this translation helpful? Give feedback.
-
Thanks for the advice about the fork option. that gives me a good hint for
starting to move the changes on my repo into the fork. Once I finish the
changes I made on my own repository I will do the fork and leave there my
changes. Most likely it will be necessary to cherry pick only a part of
them,
The things you mention like binary serialization basically cause failure of
unit tests so I just ignored them maybe deprecating them with obsolete will
suffice but I didn't had the time to put a #if conditional to put the
obsolete attribute only for .net 5 targeting, also on some projects I use
.net standard but forgot if it is 2.0 or 2.1. Will check that part.
BTW, my strategy was always to add the new targets but changing the least
on several projects without removing old targeting.
Also I did the change only on one of the solution files as I am not sure
how the other solutions work, but seem to be aimed for testing the
nugets once they are deployed.
Regards,
Paul.
…On Sun, Oct 17, 2021 at 5:16 PM Alexandre Rabérin ***@***.***> wrote:
Hello!
I'm so sorry for the delay :s
Note that you can properly fork the project by using the dedicated GitHub
feature which is the button there:
[image: image]
<https://user-images.githubusercontent.com/5725322/137646631-4b8fe974-ef32-4acc-bdd5-40c9008fc9a4.png>
All changes you will perform in a branch will then be proposed as a pull
request based on your branch and targeting the master branch of this
repository! This is I think the normal way to propose changes on a given
repository.
I'm not sure how you're using Git, if you're doing raw with command lines
or if you have some GUI or OS integrated stuff. On my side I was used to
use TortoiseGit <https://tortoisegit.org/>, but now I more on GitKraken
<https://www.gitkraken.com/>. But of course you're free to use any other
solution. The only point is be comfortable with your setup ;-)
Recently I considered opening new target for .NET 5 support for QuikGraph.
I testing a bit on a local branch. Based on the current state of code. I
noticed that the main issue I'm facing is the fact that with .NET 5 the
binary serialization is now obsolete and so it makes compilation errors.
I'm pretty sure this will be the big issue to solve in order to be able to
add .NET 5 target. Without further changes except adding the target I was
able to compile QuikGraph, QuikGraph.MSAGL and QuikGraph.Petri. Obviously
others need to be investigated and fixed because releasing anything.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#25 (reply in thread)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKWMYBHK4TPDMYRGB4TFGULUHNDKBANCNFSM4UZZOJ7A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Beta Was this translation helpful? Give feedback.
-
@KeRNeLith I think it's worth adding .NET 6, 7 targets. |
Beta Was this translation helpful? Give feedback.
-
Adding targets for .NET 5+, .NET Standard 2.1+ and .NET Core 3.1+ to QuikGraph libraries?
Beta Was this translation helpful? Give feedback.
All reactions