featzhang commented on PR #30: URL: https://github.com/apache/flink-connector-http/pull/30#issuecomment-4293169404
Hi @davidradl, Thank you for your thorough review! 🙏 I've verified that all your review comments from March 17 have been addressed in the subsequent commits (March 31): ### ✅ All Issues Fixed: 1. **"the connector is 'http' not 'rest-lookup'"** → ✅ Fixed: now using `'connector' = 'http'` for lookup source 2. **"this is not required" (README.md)** → ✅ Fixed: Quick Start content moved from README to docs folder 3. **"This connector name is not correct"** → ✅ Fixed: changed to `'connector' = 'http-async-sink'` matching `HttpDynamicTableSinkFactory.IDENTIFIER` 4. **"not correct for the apache connector"** → ✅ Fixed: connector identifiers corrected 5. **"We already document these config options"** → ✅ Fixed: removed duplicate configuration table from Quick Start ### 📋 Current State: - All review threads are marked as **resolved** ✅ - README.md remains lightweight (following Kafka connector style) - Quick Start examples added to `docs/content/docs/connectors/table/http.md` (both EN and ZH versions) - All connector identifiers verified against factory classes in code - CI passing on Java 11 & 17 Could you please re-review when you have a moment? All your feedback has been incorporated. Thank you! 🙏 -- 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]
