modified documentation to not use DataConverter incorrectly - #867
modified documentation to not use DataConverter incorrectly#867Krashcan wants to merge 1 commit into
Conversation
zawadzkidiana
left a comment
There was a problem hiding this comment.
Thanks for the doc cleanup — agreed that Javadoc referencing a mechanism that no longer applies is worse than no comment at all, since it actively misleads whoever goes looking at it next.
Looking at the current Worker.java on master, though, this specific constructor's Javadoc has since been rewritten as part of a larger change — the constructor signature itself now takes factoryOptions, cache, enableStickyExecution, threadPoolExecutor, contextPropagators, etc., and the @param options line no longer mentions DataConverter at all. So this particular one-line fix looks like it's already been superseded and won't apply cleanly against current master anymore.
If there's still stale DataConverter-via-worker-options wording elsewhere in the docs/Javadoc that you ran into, it'd be great to get an updated PR against current master for that — happy to take a look. Otherwise this one is probably safe to close as no-longer-applicable. Thanks for taking the time to send it in either way!
Generated by Claude Code
|
Thanks for catching this, @Krashcan, and sorry it sat here unreviewed for so long. Your report was accurate. The SingleWorkerOptions.newBuilder()
.setDataConverter(client.getOptions().getDataConverter())So the doc was pointing people at a knob that isn't on that dial, which is a genuinely misleading kind of stale comment — worse than no comment. Current state on The line has since been fixed, though by a different route. The constructor Javadoc now reads: * @param options Options for configuring worker.with the For what it's worth, I think plain wording was the better landing spot than the substitution proposed here, for a reason worth noting: The branch also now conflicts with I'd suggest closing this as resolved. Thank you for reading the docs closely enough to notice, and for taking the trouble to open a PR rather than just working around it — small correctness fixes to public-API docs are genuinely valuable, and I'm sorry this particular one didn't get the timely response it deserved. Please don't let the delay put you off filing the next one. Generated by Claude Code |
What changed?
Documentation for Worker constructor
Why?
It incorrectly mentioned that things like DataConverter are being passed via options which is no longer the case since release v3.0.0.
How did you test it?
No actual code change done
Potential risks
Nothing, worst case my documentation itself is incorrect.
Release notes
No
Documentation Changes
No