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

What did I get myself into... #213

Merged
merged 13 commits into from
Jun 18, 2023
Merged

Conversation

taubenangriff
Copy link
Contributor

@taubenangriff taubenangriff commented Apr 10, 2023

dependency injection is easy they said... it will be much fun they said..

@jakobharder
Copy link
Collaborator

You‘re sure you‘re not over complicating things?

@taubenangriff
Copy link
Contributor Author

no

@taubenangriff
Copy link
Contributor Author

This branch builds again at least. (Don't talk about the tests)

What underwent major changes

  • Mod loading / Mod Collection loading is done via factory
  • ModTweaker IO is different (file format stays same ofc).
  • Hooks are a little different and have to be registered in a modcollection.
  • githubrepoinfo strategies simplified: instead of a strategy pattern in the githubrepoinfo we use it where it is needed, and inject it with DI.

Problems

  • IValueConverters. Screw them.

I can't believe I did this just because I hated the GithubClientProvider with the static client so much.

@taubenangriff
Copy link
Contributor Author

Converters work now. After configuring the services, They are added to the resource dictionaries in code behind. I know, it's hacky, but it works. @jakobharder you decide, if it's worth fixing the tests, I'll do it. If it's not worth, this branch can be scrapped.

@taubenangriff
Copy link
Contributor Author

Okay, branch works and the subfolder thing is also in there. Make or break, this gonna get merged or nuked?

@taubenangriff taubenangriff marked this pull request as ready for review May 15, 2023 21:43
@taubenangriff taubenangriff merged commit 37ef709 into master Jun 18, 2023
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