<div dir="ltr">Hello, thank you for the review!<br><br>I took into account the comments and soon i will send new version<br>of the patch.<br><br>About additional macroses like EXPORT_ICU and others, as Timur<br>mentioned, seems there is no need to create them, because as I<br>noted in commentary at src/exports_icu.h - unicode/urename.h<br>file do this work (maps exported symbols to symbols with version<br>suffix, if library is build with option --enable-renaming). Also we<br>can't rely on U_ICU_ENTRY_POINT_RENAME macros, because old versions<br>of libicu don't contain this macros and unicode/urename.h has a<br>simple symbols mapping:<br>#define symbol symbol_suffix<br>So, I think, include <unicode/urename.h> is enough.<br><br>Regards,<br>Alexandr</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 22 сент. 2020 г. в 00:05, Timur Safin <<a href="mailto:tsafin@tarantool.org">tsafin@tarantool.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">: From: Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org" target="_blank">v.shpilevoy@tarantool.org</a>><br>
: Subject: Re: [PATCH] Add missed icu symbols<br>
: <br>
...<br>
: > +EXPORT(u_strlen)<br>
: > +EXPORT(u_uastrcpy)<br>
: > +EXPORT(u_austrcpy)<br>
: > +EXPORT(u_errorName)<br>
: > +EXPORT(u_getVersion)<br>
: > +EXPORT(udat_open)<br>
: > +EXPORT(udat_setLenient)<br>
: > +EXPORT(udat_close)<br>
: > +EXPORT(udat_parseCalendar)<br>
: > +#if U_ICU_VERSION_MAJOR_NUM >= 55<br>
: > +    EXPORT(udat_formatCalendar)<br>
: > +#endif<br>
: > +EXPORT(ucal_open)<br>
: > +EXPORT(ucal_close)<br>
: > +EXPORT(ucal_get)<br>
: > +EXPORT(ucal_set)<br>
: > +EXPORT(ucal_add)<br>
: > +EXPORT(ucal_clear)<br>
: > +EXPORT(ucal_clearField)<br>
: > +EXPORT(ucal_getMillis)<br>
: > +EXPORT(ucal_setMillis)<br>
: > +EXPORT(ucal_getAttribute)<br>
: > +EXPORT(ucal_setAttribute)<br>
: > +#if U_ICU_VERSION_MAJOR_NUM >= 51<br>
: > +    EXPORT(ucal_getTimeZoneID)<br>
: > +#endif<br>
: > +EXPORT(ucal_setTimeZone)<br>
: > +EXPORT(ucal_getNow)<br>
: <br>
: 3. Lets better store everything in exports.h. You can add a new<br>
: section in this file separated by a comment, that these symbols are<br>
: from ICU, to not scatter them among other exports. Also please sort<br>
: the exports in the alphabetical order, as it is asked in exports.h<br>
: comment.<br>
<br>
Agreed with Vlad here - keeping all inside of single exports.h would make <br>
this code much, much easier to maintain. I'd recommend you to return back <br>
to the original idea with using special EXPORT_ICU(name) macros which would <br>
expand to EXPORT(U_ICU_ENTRY_POINT_RENAME(name)) _iff_ not U_DISABLE_RENAMING<br>
and to EXPORT(name) otherwise.<br>
<br>
And for +#if U_ICU_VERSION_MAJOR_NUM >= 51 I'd suggest (but not insist) to <br>
create yet another wrapper EXPORT_ICU_51UP(name). This would make it looks <br>
very consistent with others. Though with 1 usage case it might be overkill.<br>
<br>
Regards,<br>
Timur<br>
<br>
</blockquote></div>