Code review comment for ~dirk.zimoch/epics-base:epicsMutexPriorityInheritance

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> ... intended for public use, which the epicsPosixMutex stuff shouldn't be ...

Ok, as internal helpers, calls like these (with an ANJ compliant name) are perfectly reasonable. I would like to see them moved to a private header. Maybe 'os/posix/osdMutexPvt.h'?

> ... I'm not convinced we need the env switch ...

Likewise. We already have the build time $(USE_POSIX_THREAD_PRIORITY_SCHEDULING) switch.

> ... I'm not sure how it helps future maintainers to understand this file ...

I'll go further. In-code comments should provide meaningful context for the code around them. This discussion would make a nice core-talk (or maybe blog) post, but is a distraction here.

> ... I think we would probably want to delay merging this until after Heinz' RTEMS-5 code has been pulled in ...

I don't like this. To my mind, we merge whichever is ready first, and the second needs to be reworked to resolve any conflicts. I think this gives a health incentive for promptness.

« Back to merge proposal