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

bedrock merged + nuget package update #39

Closed
wants to merge 1 commit into from
Closed

Conversation

drakon660
Copy link
Contributor

I moved bedrock framework parts that are being used in Tye2.

Closes #38

@drakon660 drakon660 requested a review from razvangoga as a code owner March 10, 2024 08:58
Copy link

Test Results

201 tests   201 ✅  9m 35s ⏱️
  3 suites    0 💤
  3 files      0 ❌

Results for commit 9d1119c.

@razvangoga
Copy link
Collaborator

@drakon660 not sure how i feel about this - i would try to understand first for what / why exactly does tye(2) need the bedrock package, and ideally try to replace it with something more mature (like you suggested to replace the System.Console with Specter)

what is the quality level of these classes you want to add? poc / alpha / beta / rc? do they have any tests (or at leat docs) to allow us to sanely maintain them in the future (if we pull them in - it's on us to maintain them)

@drakon660
Copy link
Contributor Author

Bedrock is different story, basically it is a bunch of helpers to extend asp.net core to act as a socket server with a few protocols implementations, it is used by a proxy service and work like a proxy that input stream is being copied to an output stream. In terms of quality it is not a valid question in this context, first you I had to assume that rest of a Tye2 is somehow completed and at least good quality that's false. There are some tests, but our integration tests cover it of course, but as I sad it can be moved. In this stage I don't think so that we should deliberate about something that is working but about something that is not. Regarding CommandLine is pretty big framework so this is different thing.

@razvangoga
Copy link
Collaborator

razvangoga commented Mar 10, 2024

first of all : this is why i opened the 2 discussions about the two libs - to understand the why and the how before actually doing anything. I've poked around tye's codebase a bit (2 of the PRs i merged at the beginning were mine) but i have no ideea:

  • why tye needs bedrock
  • how much of bedrock it actually uses (how many from all those helpers you mention)
  • what alternatives are there (MS is known for their not-invented-here mentality - there my be an open source lib that does the same and they just did not want to use it)

In terms of quality it is not a valid question

i disagree here : bedrock does not seem like something i would use if i was doing a greenfield proj

  • it's an unmaintained (last commit was 2y ago - feb '22, last functional commit was in sept '21)
  • it' hosted under Fowler's personal account and not under any official MS / dotnet / asp net organisations (nb: tye is under the dotnet org),
  • was rejected from being part of the full framework
  • the repo looks like tye's repo looked like - has some users that open issues and send PRs and get mostly ghosted by the author

There are some tests, but our integration tests cover it of course, but as I sad it can be moved

i don't want new code put in that is not covered by tests. there are enough unknowns in the current codebase - let's not introduce some more

In this stage I don't think so that we should deliberate about something that is working but about something that is not

that's why i started it as a discussion - if the result would be to stick for now to bedrock, i would rather use it via nuget than pull it as source code in here.


let's continue the discussion here please #37 - can you put there in writing what exactly is tye doing with bedrock? i would like to understand the problem better.

@drakon660 drakon660 marked this pull request as draft March 11, 2024 08:24
@drakon660 drakon660 closed this Nov 6, 2024
@drakon660
Copy link
Contributor Author

abandon

@drakon660 drakon660 reopened this Nov 6, 2024
@drakon660 drakon660 closed this Nov 6, 2024
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.

Remove Bedrock Framework
2 participants