Code review comment for ~johill-lanl/epics-base/+git/epics-base:timer-queue-fix

Revision history for this message
Jeff Hill (johill-lanl) wrote :

Hi Martin,

Sorry about delayed response; I have been focusing on another project.

I had a quick look at your branch, and all of it looks in principal good. My spelling is unfortunately egregious and of course the doc is a needed upgrade. Some of the macros I use in other codes, to ease into C++ 11, but if some of them are already in 7.0 under different names then I will need to switch to those names of course. I don't recall seeing these duplications under different names; I will need to look closer.

The API on the gnu demangler function is unfortunately tricky to use, and I had to make some adjustments in the demangler support early on, but haven't touched that code for a year or so. I haven't looked at support for other compilers such as clang, but perhaps it (clang) will be compatible to gcc. As I recall the MSVC provides names from type_info::name already in human readable form.

I have another merge request in progress which actually uses some of the c++ 11 compatibility macros you are removing. They could be added back in for that proposal of course.

I could merge, probably all of, your changes into my branch. Is that how you want to proceed?

Jeff

________________________________
From: <email address hidden> <email address hidden> on behalf of Martin Konrad <email address hidden>
Sent: Monday, April 27, 2020 11:30 AM
To: Hill, Jeff
Subject: [EXTERNAL] Re: [Merge] ~johill-lanl/epics-base/+git/epics-base:timer-queue-fix into epics-base:7.0

I have pushed a few suggestions to https://github.com/mark0n/epics-base/tree/timer-queue-fix-mk that among other things fix the failing builds with clang. I'll keep adding to this branch as I review the code.
--
https://code.launchpad.net/~johill-lanl/epics-base/+git/epics-base/+merge/382887
You are the owner of ~johill-lanl/epics-base/+git/epics-base:timer-queue-fix.

« Back to merge proposal