|
Description
Simon McVittie
2011-06-13 07:40:16 UTC
Created attachment 47896 [details] [review] [PATCH 1/8] Upgrade the type system into its own top-level section of the spec The type system can be used independently, for instance in GVariant (although GVariant's binary encoding is in fact not the same). Created attachment 47897 [details] [review] [PATCH 2/8] Split Basic and Container types into subsections, promote "Type Signatures" to be an intro The "Type Signatures" subsection is basically an introduction to the type system, so it doesn't need a heading of its own. Created attachment 47898 [details] [review] [PATCH 3/8] Define single complete types in the overview of the type system Created attachment 47899 [details] [review] [PATCH 4/8] Don't claim that all basic types work like INT32: strings don't! Created attachment 47900 [details] [review] [PATCH 5/8] Define the fixed and string-like types a bit more formally Created attachment 47901 [details] [review] [PATCH 6/8] Promote the definition of valid object paths and signatures into the type system Also remove the (double!) requirement that signatures be nul-terminated, and turn it into a note about the marshalling format. Created attachment 47902 [details] [review] [PATCH 7/8] Promote the marshalling format to a top-level section Created attachment 47903 [details] [review] [PATCH 8/8] Describe how to marshal arrays, structs, dict-entries, variants in prose (In reply to comment #1) > Created an attachment (id=47896) [details] > [PATCH 1/8] Upgrade the type system into its own top-level section of the spec > > The type system can be used independently, for instance in GVariant > (although GVariant's binary encoding is in fact not the same). Looks good. BTW: Isn't "marshalling" more correct than "marshaling"? I mean it's "labelling" and not "labeling", right? (In reply to comment #9) > BTW: Isn't "marshalling" more correct than "marshaling"? FOLDOC claims that "marshalling" is correct en_GB, and both are correct en_US. After this branch has been merged, I'd be more than happy to apply British English spellings if people prefer them :-) (In reply to comment #9) > (In reply to comment #1) > > [PATCH 1/8] [...] > > Looks good. Thanks, merged for 1.5.6. Any thoughts on the other 7 patches? (Or was this intended as positive review of all 8?) (In reply to comment #11) > > > [PATCH 1/8] [...] > > Thanks, merged for 1.5.6. Any thoughts on the other 7 patches? (Or was this > intended as positive review of all 8?) Ping? I'd like to get this merged, spec clarity is a good thing. (In reply to comment #12) > Ping? I'd like to get this merged, spec clarity is a good thing. This has now missed the boat for 1.6. How about 1.7? Comment on attachment 47897 [details] [review] [PATCH 2/8] Split Basic and Container types into subsections, promote "Type Signatures" to be an intro Review of attachment 47897 [details] [review]: ----------------------------------------------------------------- Looks fine. Comment on attachment 47898 [details] [review] [PATCH 3/8] Define single complete types in the overview of the type system Review of attachment 47898 [details] [review]: ----------------------------------------------------------------- Looks fine. Comment on attachment 47899 [details] [review] [PATCH 4/8] Don't claim that all basic types work like INT32: strings don't! Review of attachment 47899 [details] [review]: ----------------------------------------------------------------- Looks good. Comment on attachment 47900 [details] [review] [PATCH 5/8] Define the fixed and string-like types a bit more formally Review of attachment 47900 [details] [review]: ----------------------------------------------------------------- I guess this slightly duplicates information from the two big tables, but that's not necessarily a bad thing. Comment on attachment 47901 [details] [review] [PATCH 6/8] Promote the definition of valid object paths and signatures into the type system Review of attachment 47901 [details] [review]: ----------------------------------------------------------------- Fine. Comment on attachment 47902 [details] [review] [PATCH 7/8] Promote the marshalling format to a top-level section Review of attachment 47902 [details] [review]: ----------------------------------------------------------------- Looks good. Comment on attachment 47903 [details] [review] [PATCH 8/8] Describe how to marshal arrays, structs, dict-entries, variants in prose Review of attachment 47903 [details] [review]: ----------------------------------------------------------------- Also looks good. Fixed in git for 1.7.0, thanks |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.