Skip to content
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

Feat: .NET generic host #12

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

lduchosal
Copy link

Fixes issue

Add a working solution file

Description

Getting started on the .NET generic host, need working solution and project

@lduchosal lduchosal changed the title Fix: working sln file Feat: .NET generic host Apr 3, 2025
@lduchosal
Copy link
Author

lduchosal commented Apr 3, 2025

That's a lof of files, but everything I added is unit tested.
If this is too big of a change for you, I can create another project.
Let me know

@smdn
Copy link
Owner

smdn commented Apr 3, 2025

Thank you for sharing the codes that illustrate your goal.
However, I cannot merge this PR as it is because of the following reasons:

1. Changing/adding sln and csproj files is not directly related to the .NET generic host implementation in this PR.

  1. Changes to the sln and csproj files will break test and release workflows that are integrated with GitHub Actions.
  2. In my project, I do not use sln files to manage csproj files. If you need sln files, use them only in your local environment.

Use the dotnet test command to run unit tests. Or, build the project in the src/ directory first, and then run the tests in the project in the tests/ directory.

2. The changes to tests/Smdn.Net.MuninNode/Smdn.Net.MuninNode/NodeBase.cs are not directly related to the .NET generic host implementation in this PR.

While I fix the problem with this test failing, please wait a while.
(Update)Fixed in #13.

3. As we are adding a lot of files, I would like to separate the PR.

As I mentioned in my comment in #11, I will merge your code in order after separating it into some PRs.

Let's start by introducing and implementing the IAccessRule.
I was not aware that this interface would be added.
Can you write a design about IAccessRule in #11?

@smdn
Copy link
Owner

smdn commented Apr 3, 2025

Fixed NodeBase unit test failure problem in #13.
I have also released version 2.1.0, which fixes this problem.
Update your code to apply the fixes.

@lduchosal
Copy link
Author

Thank you for sharing the codes that illustrate your goal. However, I cannot merge this PR as it is because of the following reasons:

1. Changing/adding sln and csproj files is not directly related to the .NET generic host implementation in this PR.

1. Changes to the sln and csproj files will break test and release workflows that are integrated with GitHub Actions.
2. In my project, I do not use sln files to manage csproj files. If you need sln files, use them only in your local environment.

why don't you use sln ? is there a specific reason ?

@lduchosal
Copy link
Author

lduchosal commented Apr 3, 2025

Let's start by introducing and implementing the IAccessRule. I was not aware that this interface would be added. Can you write a design about IAccessRule in #11?

I did not introduce IAccessRule, this interface existed before :
src/Smdn.Net.MuninNode/Smdn.Net.MuninNode/IAccessRule.cs

# Conflicts:
#	tests/Smdn.Net.MuninNode/Smdn.Net.MuninNode.Tests.csproj
@smdn
Copy link
Owner

smdn commented Apr 4, 2025

I did not introduce IAccessRule

The IAccessRule was actually introduced by myself, as you said. I simply forgot about it. Sorry for causing unnecessary confusion.

why don't you use sln ? is there a specific reason ?

Due to problems with version control systems and other issues in the past, I stopped using sln files.
What is mentioned at the beginning of the following blog post is exactly the same as all the problems that I have encountered.

New, Simpler Solution File Format - Visual Studio Blog

When using VSCode or dotnet commands, sln files are not necessarily needed, so I have done without sln files until now.
So I have high expectations for the SLNX file. If the SLNX file brings a good experience, I would like to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants