yuqi1129 commented on PR #12399:
URL: https://github.com/apache/gravitino/pull/12399#issuecomment-5253907904

   Thanks for taking this on. The de-duplication is the right call — the five 
pages really were one page copied five times, and the link cleanup is a real 
bonus (against current `main`, this branch removes 16 broken links and adds 
none).
   
   I pushed two commits to the branch directly, since they were mechanical. 
Please review them:
   
   - `e7bef74` — the `curl` snippets on all five cloud pages left a trailing 
comma on the last property, so the JSON is invalid as copied. Also restores the 
`fs.path.config.<name>` placeholder (truncated to `<n>`), realigns the Storage 
Backends table, and repoints two anchors that no longer resolve.
   - `01c182c` — merges current `main`. The only conflict was link text on 
three links to the authentication page; I kept your wording.
   
   ## My main concern: this is trimmed a bit too far
   
   Reading it end to end, my overall impression is that the cuts went past the 
duplication and into operational detail that readers actually need.
   
   The `hadoop fs` path is the clearest example. Each page now says to put the 
properties in `core-site.xml` and "place the same jars in the Hadoop classpath" 
— but "the same jars" points back at a Maven dependency list, and the specifics 
that used to be there (which jars, which directory, and for COS the fact that 
they aren't in the Hadoop distribution at all) are now gone from the whole docs 
tree. The shared walkthrough in `how-to-use-gvfs.md` doesn't fill the gap 
either; it just says to put the required properties in the file, without 
showing them.
   
   But that's only one instance. The same pattern shows up elsewhere — version 
and environment caveats, the reason behind a scheme or a setting, the small 
warnings that stop someone an hour of debugging. Individually each looked like 
boilerplate worth cutting; together they were most of what made these pages 
usable.
   
   The distinction I'd suggest applying on another pass: text that is 
duplicated *and* recoverable from the shared pages should go, which is most of 
what you cut. Text that is duplicated but is the only place a fact is written 
down needs a home somewhere before it disappears — either once in the shared 
page, or in the per-cloud section when it genuinely differs per cloud. I don't 
think this needs many lines to fix, mostly it's about where things land rather 
than restoring prose.
   
   Happy to go through it with you or write the restored parts myself if that's 
easier — just say which you'd prefer.
   
   ## Smaller points
   
   - The PR description is now stale: it describes deleting the five cloud 
pages, but they were restored in trimmed form and only 
`fileset-catalog-index.md` is actually deleted. Worth rewriting before this is 
squashed.
   - `s3-endpoint` is `Required: Yes` in the catalog properties table and `No` 
in the Python table on the same page. If the difference is real it should be 
called out; if not, they should agree.
   


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to