|
David Ostrovsky |
|
|
Hi,
unfortunately gerrit is out of sync now, so i can not push it for review. I have some questions to this patch: 1. Test and workben directories are not used: let the old makefiles in place? 2. What is this for: CDEFS += -DDLL_VERSION=$(EMQ)"$(DLLPOSTFIX)$(EMQ)" 3. Where these versions are used? .INCLUDE : ..$/version.mk 4. Have no idea what is this for => ignored ;-) makedocpp: $(DOCPPFILES) docpp -H -m -f -u -d $(OUT)$/doc$/$(PRJNAME) $(DOCPPFILES) 5. how to handle this? TARGET=reg [...] .IF "$(COM)" == "MSC" SHL1IMPLIB= ireg .ELSE SHL1IMPLIB= $(TARGET) .ENDIF Thanks David _______________________________________________ LibreOffice mailing list [hidden email] http://lists.freedesktop.org/mailman/listinfo/libreoffice |
|
Matúš Kukan |
|
|
Hi David,
On 5 May 2012 00:21, David Ostrovsky <[hidden email]> wrote: > I have some questions to this patch: > > 1. > Test and workben directories are not used: let the old makefiles in place? Either that or remove the directories completely. I've been keeping them untouched. > 2. > What is this for: > CDEFS += -DDLL_VERSION=$(EMQ)"$(DLLPOSTFIX)$(EMQ)" probably nothing > 3. > Where these versions are used? > .INCLUDE : ..$/version.mk nowhere I guess > 4. Have no idea what is this for => ignored ;-) > makedocpp: $(DOCPPFILES) > docpp -H -m -f -u -d $(OUT)$/doc$/$(PRJNAME) $(DOCPPFILES) hmm, this is probably useless, so ok to ignore > 5. how to handle this? > TARGET=reg > [...] > .IF "$(COM)" == "MSC" > SHL1IMPLIB= ireg > .ELSE > SHL1IMPLIB= $(TARGET) > .ENDIF You don't need to do anything about this. 'i' prefix is default for .lib file. Otherwise you would need to touch RepositoryFixes.mk. Your patch looks good, only few comments: - AFAICS rdbedit is not delivered so can't be used (see prj/d.lst) - there was no regcpp library, only archive used to create reg library (which is already registered) -> I think that it has worked for you because you have not done build from clean ? - maybe registry_helper could be called registry_common or just registry ? But I don't care too much. Otherwise it could be good, though I can't get it build on windows and it does not print reasonable error. ireg.lib is missing but I don't know why. Thanks, Matus _______________________________________________ LibreOffice mailing list [hidden email] http://lists.freedesktop.org/mailman/listinfo/libreoffice |
|
Matúš Kukan |
|
|
On 5 May 2012 02:47, Matúš Kukan <[hidden email]> wrote:
> Otherwise it could be good, though I can't get it build on windows and > it does not print reasonable error. > ireg.lib is missing but I don't know why. Oh, and of course there is this sometimes painful DLLPUBLIC change. You need to do something as in http://cgit.freedesktop.org/libreoffice/core/commit/?id=cf77e8a0b9dc26d5007c76388c3f09231f048bdd Create registrydllapi.h file, use it, deliver, ... Thanks, Matus _______________________________________________ LibreOffice mailing list [hidden email] http://lists.freedesktop.org/mailman/listinfo/libreoffice |
|
David Ostrovsky |
|
|
Hi Matúš,
thank you very much for your review and your help (on IRC). I updated my patch accordingly. Ciao David On 05.05.2012 16:46, Matúš Kukan wrote: > On 5 May 2012 02:47, Matúš Kukan<[hidden email]> wrote: >> Otherwise it could be good, though I can't get it build on windows and >> it does not print reasonable error. >> ireg.lib is missing but I don't know why. > Oh, and of course there is this sometimes painful DLLPUBLIC change. > You need to do something as in > http://cgit.freedesktop.org/libreoffice/core/commit/?id=cf77e8a0b9dc26d5007c76388c3f09231f048bdd > > Create registrydllapi.h file, use it, deliver, ... > > Thanks, > Matus _______________________________________________ LibreOffice mailing list [hidden email] http://lists.freedesktop.org/mailman/listinfo/libreoffice |
|
David Tardon |
|
|
On Sat, May 05, 2012 at 11:09:33PM +0200, David Ostrovsky wrote:
> Hi Matúš, > > thank you very much for your review and your help (on IRC). > I updated my patch accordingly. Pushed, thanks! D. _______________________________________________ LibreOffice mailing list [hidden email] http://lists.freedesktop.org/mailman/listinfo/libreoffice |
| Powered by Nabble | Edit this page |