Skip to content

Conversation

@kevin-montrose
Copy link
Collaborator

Maintains roughly the same internal structure, including tests and benchmarks.

Targets .NET Standard & Core. C# 7.2.

No work on optimization or "idiomizing" yet, just getting to parity.

There are considerable changes to repo structure here, so I'm not expecting this to get merged. Rather it's a point for discussion and review.

…l structure, including tests and benchmarks. No work on optimization or "idiomizing" yet, just getting to parity.
@clipperhouse
Copy link
Owner

Looks good, ran successfully on my machine (Mac, .Net Core 2.1). One test fails.

For merging, do we want .Net in the same repo as Go? Might be kinda fun and good for sharing test data.

Happy to chat about how we’d like to organize the code in a more .Net-ish way.

Copy link
Owner

@clipperhouse clipperhouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment on PR. LGTM and I ran it successfully on my machine. I don’t know if the .Net code should live in this repo or whether we want to rewrite.

@kevin-montrose
Copy link
Collaborator Author

Gonna tell me which test fails?

@clipperhouse
Copy link
Owner

The Go package needs to be at the top level, moving it down under go/ will break import paths.

@clipperhouse
Copy link
Owner

This is looking really good.

@clipperhouse
Copy link
Owner

We can merge this in if you are happy with it. As I mentioned, we can’t have the go/ subdirectory — so I’ll let you do whatever git fu you need to do.

Might just cherry pick the .Net commits into a new branch — or even just copy pasta the dotnet directory as a new commit against master, if you don’t care about the commit history.

@kevin-montrose
Copy link
Collaborator Author

I've still got a few more rough edges I want to sand off, then I'll probably just close this and submit a new PR w/o history. Want to chat about directory structure sometime, maybe this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants