On Mon, Dec 07, 2020 at 05:11:42PM +0800, Shi Lei wrote: > On 2020-12-05 at 02:55, DanielP. Berrangé wrote: > >On Fri, Sep 04, 2020 at 11:34:52AM +0800, Shi Lei wrote: > >> V1 here: [https://www.redhat.com/archives/libvir-list/2020-June/msg00357.html] > >> > >> Differ from V1: > >> > >> * Move the generator into scripts/xmlgen and rename it 'xmlgen'. > >> > >> * Declare virXMLChildNode and virXMLChildNodeSet in libvirt_private.syms. > >> > >> * Replace VIR_FREE with g_free and VIR_ALLOC[_N] with g_new0. > >> > >> * Adjust virReportError to avoid unnecessary translation. > >> > >> * Remove the macro VIR_USED and use G_GNUC_UNUSED to declare arguments. > >> > >> * When parsing string member, assign value to it directly instead of > >> using middle variable. > >> > >> * Don't set libclang_path. Just use python-clang's default setting. > >> > >> * Use virEscapeString for escaping xml characters. > >> > >> * Enable directive 'genformat' with a parameter to support separation mode. > >> > >> * Add directive 'xmlswitch' and 'xmlgroup' to support discriminated unions. > >> > >> * Allow directive 'array' and 'specified' to carry with a parameter, > >> which specifies its counterpart explicitly. > >> > >> * Enable directive 'xmlattr' with path. > >> > >> * Add directive 'formatflag' and 'formathook'. > > > >I'm very sorry for the extremely long delay in reviewing this new > >series. > > > >Overall I like this series and would like to see how we can focus on > >getting at least part of it merged. It is interesting to see how it > >is forcing a separation of the parsing and validation of data. I > >also like that it is making the code more consistent in style. > > > >The main blocker I think is that we need to temporarily commit the > >generated files to git, and not run the generator during normal > >builds, until we ditch RHEL-7 support in April 2021. This should > >be quite easy to deal with. > > > >The next thing is that wierd need to insert a "_NONE" field in to > >all the enums. I'm not understanding what reqiures this, but I think > >we need to come up with a different solution to whatever the problem > >is. It looks like this only affects the DomainGraphics conversion > >in your series. So we could start by only merging the Network conversion > >patches, until we figure out a solution to avoid adding _NONE fields. > > > >That's it for the main blockers to merge from my POV. > > > >After that I think a priority is to get some test coverage of the > >generator script, and to add the docs about the how the generator > >annotations work. This can be done after the initial merge. > > > >Then there's a large ongoing work to actually convert everything, > >but there's no rush to finish that, as you've shown that we can > >do it incrementally. > > > >So if you want to re-post an update of the series with the few > >blocking items addressed, then personally I'd look at trying to > >merge that unless other people have objections they want to > >raise that can't be solved after merging the initial support. > > > >Regards, > >Daniel > >-- > > Thanks for reviewing this series. > > For the first blocker, I think we can wait until April 2021, since we have spent a lot time on it. > > According to your comment, I think I can make some preparations for the initial merge during > these 4~5 months. > > (1) Implement some testcases for the generator to make sure its proper functions in the future. > > (2) Add some docs to explain and demonstrate this generator. My English is not well, so I will > try my best to do it and I may need a bit long time. > > (3) For the 'default' item of enums, I think we can add an extra directive to direct the generator > how to generate code for enum, as I mentioned in the last email. Or we can continue to talk about > other solutions. > > (4) Next series will only include conversion about network, so that we can focus on the docs, > testcases and this generator itself. > > I plan to finish these jobs and post the next series in the next month, which will be still a RFC. > So you and other people can have sufficent time to review and comment it and I will have > sufficent time to improve it :-) Ok, if you're happy to take the time until April, that's fine with me too. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
