ctargett commented on pull request #1869: URL: https://github.com/apache/lucene-solr/pull/1869#issuecomment-696366569
I'm a little confused what's going on here at this point. It seems the latest commits increase the mixing of styles, which as I said is fine with me in general, but sort of counter to what you said you wanted to do. There are 3 tables in `charfilterfactories.adoc` for example, and two are now changed to remove the `[options="header"]` and add a blank line between header row and data rows, but the header param remains on the 3rd table which also includes the `%autowidth.spread,width="100%"` parameters which I thought I showed aren't necessary and are already the default. Despite all that, the `asciidoc-syntax.adoc` is updated to imply the best practice is defining the autospread, width, and header params explicitly, even though other tables in this PR were changed to definitely not do that. It stops short of saying it's a best practice, though, it just points out that one might see it (when before it was more direct in what it recommended writers to do). So, I'm no longer sure what's really being achieved here. Removing TODOs that aren't needed anymore because they're holdovers from PDF days is a laudable goal and helpful cleanup, but is it any more clear now what both the internal docs and examples within the content would recommend someone do to define tables? I know that wasn't the original intent, but it got turned into a goal and now it's not doing that. At this point I'm not sure if I should approve the PR (since I don't think it's ultimately that big a deal) or if I should look to hold the PR to some level of internal consistency? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
