kbendick commented on code in PR #4665:
URL: https://github.com/apache/iceberg/pull/4665#discussion_r861439484


##########
docs/flink/flink-getting-started.md:
##########
@@ -70,7 +70,7 @@ Step.3 Start the flink SQL client.
 
 We've created a separate `flink-runtime` module in iceberg project to generate 
a bundled jar, which could be loaded by flink SQL client directly.
 
-If we want to build the `flink-runtime` bundled jar manually, please just 
build the `iceberg` project and it will generate the jar under 
`<iceberg-root-dir>/flink-runtime/build/libs`. Of course, we could also 
download the `flink-runtime` jar from the [apache official 
repository](https://repo.maven.apache.org/maven2/org/apache/iceberg/iceberg-flink-runtime/).
+If we want to build the `flink-runtime` bundled jar manually, please just 
build the `iceberg` project and it will generate the jar under 
`<iceberg-root-dir>/flink/vx.xx/flink-runtime/build/libs`. Of course, we could 
also download the `flink-runtime` jar from the [apache official 
repository](https://repo.maven.apache.org/maven2/org/apache/iceberg/iceberg-flink-runtime/).

Review Comment:
   If we're updating this, the link 
https://repo.maven.apache.org/maven2/org/apache/iceberg/iceberg-flink-runtime/ 
is no longer correct.
   
   The Flink 1.14 link would be 
https://repo.maven.apache.org/maven2/org/apache/iceberg/iceberg-flink-runtime-1.14.
   
   Maybe it would be better to specifically mention the fact that there's a 
runtime jar per Flink version?
   
   Something like
   ```
   If we want to build the `flink-runtime` bundled jar manually for a specific 
Flink version, such as v1.14, please just build the `iceberg` project and it 
will generate the jar under 
`<iceberg-root-dir>/flink/v1.14/flink-runtime/build/libs`. Of course, we could 
also download the `flink-runtime-1.14` jar from the [apache official 
repository](If we want to build the `flink-runtime` bundled jar manually, 
please just build the `iceberg` project and it will generate the jar under 
`<iceberg-root-dir>/flink/vx.xx/flink-runtime/build/libs`. Of course, we could 
also download the `flink-runtime` jar from the [apache official 
repository](https://repo.maven.apache.org/maven2/org/apache/iceberg/iceberg-flink-runtime-1.14/)
   ```
   
   I don't love the phrasing or the usage of pronouns (we) in this, and the 
instructions should probably mention building with the flinkVersions flag 
specified, but given that the current link is a valid link (but only for older 
code and likely won't work properly), it shouldn't be used.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to