ML code reviews

Started by names_are_hard, July 21, 2022, 03:42:42 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

names_are_hard

Hello!  It's hard to find people with the ML knowledge to do code reviews, so perhaps devs on here would like to arrange something?  We could swap reviews for changes.

I have two recent changes that I'd really appreciate a review for:
- module changes that make cache hack usage compatible with Digic 678X (and fix a bug in the build system that affects all ML repos): https://github.com/reticulatedpines/magiclantern_simplified/compare/dev...d45_cache_fix
- working but minimal framework for MMU memory patches on Digic 678X cams: https://github.com/reticulatedpines/magiclantern_simplified/compare/dev...mmu_investigation

The MMU commits are kind of ugly and need cleaning up before merging, I'm not asking for a full review there but some validation that the general approach is sound.  Testing on cams isn't required, although the D45 branch should make builds that work on older cams (this has had some testing: thanks Walter!).

I'll review your weird hacks in exchange!

theBilalFakhouri

Unfortunately I am not that experienced to a point which I can do reviews (I barely can code), so can't help with this.

I don't know how much time it will take to review a code . . well, probably g3gg0 might do it (if he still remember ML code and has some time), he seems not active on forum, asking him directly could be a good idea (Discord), or asking him if he can review code in general from time to time.

names_are_hard

Ah well, thanks for considering it :)

Reviewing can be quick, but can be slow.  The cache hack change is small and easy, I think an hour or less.  The MMU work is larger and more complicated, several hours minimum, so might take a few days of spare time.

Sounds like I will have to more carefully consider my own code - at least it's good to know.