RocMarshal commented on PR #27897: URL: https://github.com/apache/flink/pull/27897#issuecomment-4275386049
Thanks @snuyanzin for the attention and reminder, @featzhang for the response. Please let me to add some information from my perspective at the time when reviewing the PR, and sorry for missing that earlier, which may have caused confusion: I had also considered whether we needed a new FLIP to track this parameter. However, based on the following reasoning, I proceeded with the merge: - A: As @featzhang mentioned, this new parameter belongs to a sub-component or a secondary dimension. - B: Following from point `A` I looked for recent, similar changes ([1], [2]) and checked whether they introduced new FLIPs. Since none of them did, I referenced their development and review pattern, reviewed accordingly, and merged this PR. That said, if the introduction of this parameter feels too abrupt, I’m happy to revert it immediately. We can then reintroduce this change later, strictly following the specification you outlined. To ensure consistency, I've incorporated this into my review checklist. Going forward, I'll follow the specification closely to avoid similar issues. Any input is appreciated! [1]https://issues.apache.org/jira/browse/FLINK-38372 [2]https://github.com/apache/flink/blob/ee827dfcebf09aad25f26782e865027e8639e04d/flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporterOptions.java#L117 -- 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]
