Fix memory usage and chunkcache#18
Conversation
Removed deployment step to Testers Repo from workflow.
|
@UplandJacob If you could test the |
Still seems to be a bit crazy @Trainboy15
maybe resetting cache more often or not saving so many things to it? |
|
I agree, it prob shouldent use 24GB, but it seems at least a bit better than 36GB |
|
|
Yes, I know I'm still trying to fix
…Sent from my Apple Watch
|
|
K it should be fixed... use the latest bulid after it gets done |
|
@bobhenl If you could test it now that would be great |
|
where can be the build downloaded btw, I don't see it in actions |
Removed redundant job configuration for unit tests and updated the path for uploading test reports.
|
💀, I might have fixed it, let me know |
UplandJacob
left a comment
There was a problem hiding this comment.
Looks good so far! I've really needed to do a pass like this for a while but haven't had the time, so thanks for the help. I know this PR is still a draft, but I have some random comments to think about:
- Make sure you're not adding any new trailing whitespace. I've got some cleanup to do, but I don't need more 😭.
- Please don't mess with the feature loading in this PR since that involves a bunch more work to make sure everything can be reloaded.
- Please make a separate PR for the update detector. It looks good; I just can't comment on both at the same time.
I really like having some tests in our workflow; that will be nice for validation.
| } | ||
|
|
||
| public void setChunkInjector(tf.tuff.netty.ChunkInjector injector) { | ||
| if (injector == null) { |
There was a problem hiding this comment.
Since it's just a single null check, the return in a single line without a block will look cleaner. I'm trying to standardize this in our codebase, so it would be great if you could do it everywhere.
There was a problem hiding this comment.
Again, make sure these are removed before we can merge.
There was a problem hiding this comment.
How would you like me to resolve this, just remove them or build the newest AS this
There was a problem hiding this comment.
Just remove the jars from this branch. Our CI will automatically update the correct jar after merging.
There was a problem hiding this comment.
Thanks for the tests! I'll have to test this out. (uh, test-ception lol)







IDK if it works put it compiled so...
Also u might want to add back the deploy to testers repo, but i had to remove it