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

Port net-ipfs-api to .net core #25

Closed
wants to merge 4 commits into from
Closed

Port net-ipfs-api to .net core #25

wants to merge 4 commits into from

Conversation

lvermeulen
Copy link

Unit tests were changed to XUnit.

@jeremy-ellis-tech
Copy link
Owner

Much appreciated changes. Especially around the C# 6.0 features.

However, some changes move away from well established code conventions.

For example:

  • Implicitly typed local variables where it is not clear what the type is from either side of an assignment.
    ie. HttpContent content = await ExecuteGetAsync(...); vs var content = await ExecuteGetAsync(...);
  • static/const member in ALL_CAPS_SNAKE_CASE instead of PascalCase
  • Using primitive alias instead of class name for accessing static members. ie. string.Equals(..) vs String.Equals(..).

@lvermeulen
Copy link
Author

Ah. I follow CoreFX guidelines myself - https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/coding-style.md - but I recognize I may have transgressed even those. I will fix and resubmit.

@timothyparez
Copy link
Contributor

If we're going to have .NET core support, shouldn't it be in a separate branch?
If we want to keep everything in one branch we should target .NET Standard instead.

@lvermeulen
Copy link
Author

@timothyparez That's the general idea.

@lvermeulen lvermeulen closed this Oct 16, 2017
@lvermeulen lvermeulen deleted the netcore branch October 16, 2017 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants