<div dir="ltr">Hi Tom,<br><br>Thank you for the changes, looks better with the \parksip removal — but<br>strangely a \parskip of about 2em seems to have been introduced, which<br>is likely the origin of the change in the number of overfull hboxes. It<br>should not be a problem, but in section 4.7 on page 18, the indentation<br>following the example (text starting by "both the single value of")<br>would better be removed (e.g. with a \noindent).<br><br>I also noticed a bit lately that the "eq_FK5" value of the "system"<br>attribute is quoted in 2 locations, in sections 3.1 (Example) and 3.4<br>(in the "equinox" attribute) — but since this value is deprecated, it<br>would better be replaced by the recommended "FK5" keyword.<br><br>Cheers, François<br><br>==> On 2023-11-09 14:18+0000,<br>    Tom Donaldson <<a href="mailto:tdonaldson@stsci.edu" target="_blank">tdonaldson@stsci.edu</a>> wrote:<br><br>>Hi François and Markus,<br>><br>>Thanks François for the detailed review and Markus for the comments.<br>>Based on this discussion I made the changes below and included them in<br>>pull request <a href="https://github.com/ivoa-std/VOTable/pull/50" target="_blank">https://github.com/ivoa-std/VOTable/pull/50</a>.  Please add<br>>your review comments to the PR or on this e-mail thread.<br>><br>>- Updated the Appendix B title to say V1.5<br>>- Removed space from xtype="circle" in section 4.7<br>>- Explicitly listed current vocabularies for TIMESYS timescale and<br>>refposition<br>>- Removed \parindent=0pt to make paragraph breaks more apparent<br>>- Added reference to architecture figure in section 1.4 to encourage<br>>better placement of the figure<br>><br>>In removing \parindent=0pt I noticed that we went from 35 to 32<br>>overfull hboxes. Though I'm not sure that's necessarily an<br>>improvement, I didn't notice any new display problems in the draft<br>>pdf. This look does seem consistent with a couple other docs I looked<br>>at (DataLink and TAP). Please have a look for yourself at the pdf<br>>artifact generated for this PR<br>>(<a href="https://github.com/ivoa-std/VOTable/suites/18051624563/artifacts/1039567030" target="_blank">https://github.com/ivoa-std/VOTable/suites/18051624563/artifacts/1039567030</a>).<br>><br>>Regarding whether some attributes of COOSYS should be mandatory to be<br>>consistent with TIMESYS, I've added a comment to issue 23<br>>(<a href="https://github.com/ivoa-std/VOTable/issues/23#issuecomment-1803341737" target="_blank">https://github.com/ivoa-std/VOTable/issues/23#issuecomment-1803341737</a>),<br>>but still think we should maintain backward compatibility until<br>>version 2.0.  <br>><br>>Please comment that issue if clarification is needed. If you'd to add<br>>something to this version warning that certain attributes may become<br>>required, please suggest wording here on this PR.<br>><br>>Thanks,<br>>Tom<br>><br>><br>>> On 11/7/23, 2:00 PM, "apps on behalf of Markus Demleitner"<br>>> <<a href="mailto:apps-bounces@ivoa.net" target="_blank">apps-bounces@ivoa.net</a> <mailto:<a href="mailto:apps-bounces@ivoa.net" target="_blank">apps-bounces@ivoa.net</a>> on behalf of<br>>> <a href="mailto:msdemlei@ari.uni-heidelberg.de" target="_blank">msdemlei@ari.uni-heidelberg.de</a><br>>> <mailto:<a href="mailto:msdemlei@ari.uni-heidelberg.de" target="_blank">msdemlei@ari.uni-heidelberg.de</a>>> wrote:<br>>> <br>>> Hi François,<br>>><br>>> On Tue, Nov 07, 2023 at 06:02:38PM +0100, Francois Ochsenbein wrote:<br>>>  <br>>> > * section 3.5 (on TIMESYS): shouldn't the list of allowed timescale<br>>> > values be listed, as are the system COOSYS values? The same remark<br>>> >  <br>>> <br>>> In principle, I'm not overly wild about in-document vocabularies;<br>>> vocabularies are designed to grow (and to have labels and<br>>> descriptions, without which they're not terribly useful). But you're<br>>> right, if we do it in one place, we should do it in the other, too.<br>>> <br>>> I'll leave it to the editor to decide whether or not to put it<br>>> <br>>> At the time or writing, this vocabulary includes the terms<br>>> % GENERATED: !vocterms timescale<br>>> % /GENERATED<br>>> <br>>> near the current line 809 and running make generate. For<br>>> TIMESYS/@refposition, I'd say there current cross reference to COOSYS<br>>> should be enough (or perhaps add "see there for a snapshot of the<br>>> defined values as of the release of this document.<br>>>   <br>>> > applies on the list of refposition, listed for COOSYS, but not for<br>>> > TIMESYS (both share the same list). It looks also strange to me<br>>> > that the system attribute in COOSYS is optional, while the<br>>> > timescale attribute in TIMESYS is required — shouldn't this be<br>>> > harmonized ?  <br>>> <br>>> Yes, it should. The question is how to do that. Making<br>>> COOSYS/@system mandatory will make some VOTables invalid, and we're<br>>> officially not supposed to do that in a minor version.<br>>> <br>>> I suppose it would be a reasonable and defendable policy if we said<br>>> now that we reserve the right not make it mandatory two minor<br>>> versions down the road or so.<br>>>   <br>>> > And last, for the comfort in reading the document, some additional<br>>> > space between the paragraphs (the \parskip value) would be nice.  <br>>> <br>>> Since VOTable sets<br>>> <br>>> \parindent=0pt<br>>> <br>>> (which, frankly, I'd probably drop if I were the editor) you are<br>>> right that discerning paragraphs is not always simple. Hence, having<br>>> a bit of parskip is indeed wise, and I'd totally go for<br>>> <br>>> \parskip=0.5ex<br>>> <br>>> -- that's also a lot less invasive than re-introducing the parskip<br>>> (which will probably result in overfull hboxes).<br>>> <br>>> -- Markus  <br>><br><br></div>