Hi Tobias, Thanks for the thorough code review! I'm pretty swamped for the next couple of days, and I'll get the patch updated as you requested this weekend.
> I don't understand this change. First, I don't see any reason why this > should get modified. And by modifying it, a simple "git blame" will also > point to the "wrong" commit. > > But also otherwise: it does not save the on the number of lines and it > also breaks the current pattern: > > > First line – name plus classification. > > Second line – return value, Fortran standard > > Third line – functions pointers > > Fourth line – arguments > > > I have to admit that we haven't been super consistent and the first two > lines are often also in one, but mostly (but not always) the function > pointer are on a line of their own. > > > Thus, I think we can break this pattern - but, in particular, I wouldn't > do so for the existing code, unless there is a good reason for doing so. You're absolutely right that the best way to keep changes minimal is to just rename the `*resolve*` function. The challenge is I used clang-format (with the `contrib/clang-format` file) for formatting. I know it's just a recommendation, but it's super helpful for keeping the code style consistent, and doing that manually is a pain. For the current pattern section, I think it'd be better to refactor the `add_sym*` function and use a struct to group the arguments. clang-format only touched the lines I changed in my patch, so the rest is unaffected. Does that make sense? If you're really set on the style for that part, I can definitely make the change. Yuao
