-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[improve][fn] add docs for supporting reading config options from files in Python runner #544
Conversation
…es in Python runner
Hi @tpiperatgod, I submit this PR since it's more convenient to show you my thoughts and preview. PreviewDoc changes explanations2️⃣3️⃣ in the image above Reasons:
1️⃣ Consolidate the note into running text to accommodate the new structure. I've also made other changes like updating all future tense to present tense since "Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true" is a rule in technical writing. Question4️⃣ Seems that the explanations in this box is contradictory. The above indicates "--config-file" override "command-line", but the below says the opposite. Can you clarify a bit and give some examples? Thank you! |
lgtm And for question 4: I made a mistake in the first sentence, it should be that the parameters passed in from the command line have a higher priority. Thank you for the review and the advice on technical writing! |
@tpiperatgod thanks for your quick response!
Does that mean I can delete the 1st sentence and only keep the 2nd one? |
Yes |
Done, PTAL. |
@momo-jun @Huanli-Meng could you please help review? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general, with a couple of suggestions.
site2/website-next/yarn.lock is checked in unintentionally. Reverting... |
@tisonkun Oops! Thank you! |
This PR adds docs for apache/pulsar#18951. It is based on the tech inputs provided by @tpiperatgod in #541
doc
cc @D-2-Ed for editorial review, thanks!