On 2012-Oct-6 10:09 , Programmingkid wrote:
I honestly see no reason to replace open-dev with execute-device-method. My original patch is just fine.
In Forth, one of the general rules of programming is "fewer words is better". In particular, "string execute-device-method" is better than "open-dev dup >r if abort else string r@ call-method then".
The kind of review comments you've been getting here (and mostly dismissing) are normal in any shared engineering project. This is part of the normal give-and-take to produce a body of code that everyone is comfortable maintaining.
When examining someone else's forth code, if I see a long complicated sequence where a shorter one would have sufficed, I generally assume there was a necessity of using the longer one, and look to see what that necessity was. That can cause me to waste time.
As to the overall issue of adding get-time to the top-level dictionary. I don't know what the rules are in Openbios, but in Openboot, the general assumption is you _don't_ add words to the top level dictionary without an ARC (architecture review committee) case, and in the past, sending it to the IEEE 1275 committee as well. The preference is that anything in the top-level dictionary _should_ be documented in the 1275 specification or one of its updates.
That was back in the days where we wanted to keep various implementations of openfirmware inter-compatible. No longer a big issue, but in general it would be worthwhile determining what the policy of this particular codebase is before submitting changes.