-
Notifications
You must be signed in to change notification settings - Fork 267
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
Expand Linux/Mac support #61
Conversation
lextm
commented
Feb 22, 2019
- Automatic resolution of libpcap so that developers do not need to do any extra setup (like mapping libpcap to wpcap).
- Converted the test project to SDK based. It now targets both .NET Core 2.1 and .NET Framework 4.6.1.
- Added Azure Pipelines definition to enable CI on Linux and Mac.
Cannot reproduce the issues reported by AppVeyor on a Windows Server 2016 machine. Not sure if that's related to your CI script (which I couldn't find inside the repo). |
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.
Hi @lextm, thanks for this great pull request. I haven't been using sharppcap much myself for the past year or so and have been waiting for the dotnet guys to figure out how to handle this. They are working on it but its taking them a long time. This looks like a great approach and was kind of what I was thinking of doing if I got back to using the library and ran into this again.
Beyond the comments I made in the commit I had a few other thoughts,
-
AirPcap wasn't touched. I added support because they were kind enough to send me a kit but it looks like Riverbed has discontinued the AirPcap product. https://support.riverbed.com/content/support/eos_eoa/steelcentral-cascade-opnet/AirPcap-NX_Classic-TX.html. We could consider removing, or at least deprecating AirPcap support, I'm not sure anyone is using it.
-
Should we set up auto CI using azure pipelines and add a badge to the top level project? I'm a big fan f CI systems.
-
The configuration for appveyor is in the project, not in the .yml file. I'd like to move it over but I'm not an appveyor expert. We could always move away from appveyor to Azure pipelines if you think that makes sense.
-
I'm looking for additional maintainers of the project, if you were interested please let me know and I can add you to the team here on GitHub.
Cool. I will try to address the comments over the weekend. |
Most of things planned are implemented. Thanks for the kind words, but I have no plan to become maintainer of this project (as I already have several under my umbrella). Let's see how much more I might help you out. |
Hi @lextm is this ready for re-review? I'd love to get this merged in. |
@lextm, reviewed and it looks good, merged to master! |
And thanks again for the PR, this is a great improvement. |