On Mon, Oct 23, 2017 at 08:44:07AM +0000, Enrico Forestieri wrote: > On Sun, Oct 22, 2017 at 09:36:56PM -0400, Richard Heck wrote: > > On 10/22/2017 06:19 PM, Scott Kostyshak wrote: > > > On Sat, Oct 14, 2017 at 06:37:06AM +0000, Jürgen Spitzmüller wrote: > > > > > >> Also, I think we should consider Günter's lyx2lyx patch [1], but I > > >> didn't have time to thoroughly review it myself. > > > Will anyone have time to take a look at the patch by Wednesday? > > > > I could look at the code from a code-triaging point of view, but I have > > not followed > > the controversy about this, so someone would need to tell me what the > > code is supposed > > to do. > > I don't think this is a change that should be performed at a RC1 stage, > frankly. It is too risky and of dubious utility.
Thanks for bringing up this concern. The patch is not trivial. Further, the patch only deals with backwards compatibility. From what I recall, we place a higher importance on forward conversion, and although we do make efforts to provide good backward compatibility, I believe that we have at times knowingly not implemented certain functionality in our backwards conversion. Indeed, our "Development" manual covers this: While the conversion routine is required to produce a document that is equivalent to the old version, the requirements of the reversion are not that strict. If possible, try to produce a proper reversion, using ERT if needed, but for some features this might be too complicated. In this case, the minimum requirement of the reversion routine is that it produces a valid document which can be read by an older LyX. If absolutely needed, even data loss is allowed for the reversion. The current code (without the patch) clearly already satisfies the "minimum requirement". All this to say: I don't think the patch is too important for 2.3.0 and in my opinion I'm fine if we do not put it in. Since I don't think the patch is critical, and since we are hopefully a couple of days from going forward with a freeze for RC1, I propose that we should only consider this patch if a developer gives a "very confident" +1, and if we have an extensive test suite. Although an extensive test suite cannot catch all potential issues, it can certainly help us be more confident for a patch like this. I cannot confidently review the patch, but if another developer carefully reviews the patch and gives a confident +1, and if Günter has time to help, I can create a test suite of many different scenarios. This would consist of different .lyx files created by current LyX 2.3.x, and we would check the export to 2.2.x with and without the patch. We could also check the loading of the resulting files in e.g. LyX 2.2.x, and the LaTeX output from 2.2.x. There are a lot of "if's" above and we have a short amount of time, so I'm not sure it's worth it at this point. But I will leave it to others to decide. If another developer thinks it is worth it enough to spend time carefully reviewing the patch, then I volunteer to spend time working on the test suite. Scott
signature.asc
Description: PGP signature
