Subject: |
Re: [Merge] ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into epics-base:7.0 |
From: |
Andrew Johnson via Core-talk <[email protected]> |
To: |
[email protected] |
Date: |
Fri, 10 May 2019 17:19:45 -0000 |
Why not just replace the #if lines with if (...) { and leave the bodies of the functions untouched? That would make it easier for reviewers to confirm that you aren't changing the logic at all. As it is we have to understand both the original logic and what you've replaced it with and convince ourselves that their behavior will be the same, but using run-time detection instead of compile-time. Modern compilers should generate the same code for both versions so human readability is more important than conciseness.
I know that some people don't like having multiple return statements in a function; personally I disagree with that position, especially in simple cases like these where there are no resources that need releasing.
--
https://code.launchpad.net/~dirk.zimoch/epics-base/+git/epics-base/+merge/367253
Your team EPICS Core Developers is requested to review the proposed merge of ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into epics-base:7.0.
- References:
- [Merge] ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into epics-base:7.0 Dirk Zimoch via Core-talk
- Navigate by Date:
- Prev:
Re: [Merge] ~dirk.zimoch/epics-base:raspberryPi into epics-base:7.0 Andrew Johnson via Core-talk
- Next:
Jenkins build is unstable: epics-7.0 » linux64 #127 APS Jenkins via Core-talk
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
<2019>
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
[Merge] ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into epics-base:7.0 Dirk Zimoch via Core-talk
- Next:
Re: [Merge] ~dirk.zimoch/epics-base:dynamicVxWorksVmeFunctionBinding into epics-base:7.0 Dirk Zimoch via Core-talk
- Index:
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
<2019>
2020
2021
2022
2023
2024
|