Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Conversation

@kitlith
Copy link
Contributor

@kitlith kitlith commented Nov 16, 2020

NOTE: current commit only stubs ForgeHooksClient, just to get an idea of what this would look like.

Alternative to #202, basically a dead simple approach to reducing conflicts by adding more context.

Somewhat tedious to setup, but there's no underlying complexity that could bite us in the ass later.

Also introduces an annotation to mark stubbed items so that we can detect them in other tools (like patcher?)

@kitlith kitlith marked this pull request as ready for review November 17, 2020 06:57
@kitlith kitlith requested a review from TheGlitch76 as a code owner November 17, 2020 06:57
@kitlith kitlith requested a review from rikka0w0 November 17, 2020 06:57
@kitlith kitlith mentioned this pull request Nov 17, 2020
@cittyinthecloud
Copy link
Contributor

cittyinthecloud commented Nov 17, 2020

AFAICT All functionality that these classes had before is still present. Maybe add a comment at the top explaining not to try to use them in other modules, part of the appeal of codegen for me was that it made it pretty clear that the classes weren't meant for us.

@kitlith
Copy link
Contributor Author

kitlith commented Nov 17, 2020

huh, i thought they all ready had comments at the top. at least a couple of them do. I'll add the comments.

I'm still open to switching back to codegen if everyone else prefers codegen, but my personal scales have been tipped away from it and the other people I've asked have been non-committal, so I just. made a decision.

Copy link
Member

@TheGlitch76 TheGlitch76 left a comment

Choose a reason for hiding this comment

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

concept looks good, will merge later

@TheGlitch76 TheGlitch76 merged commit 8a21d78 into PatchworkMC:master Nov 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants