Skip to content
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

Update cli.rst #13774

Closed
wants to merge 1 commit into from
Closed

Update cli.rst #13774

wants to merge 1 commit into from

Conversation

xmaniic
Copy link

@xmaniic xmaniic commented Mar 20, 2024

All pull requests must have an associated issue in the issue tracker. If there
isn't one, please go open an issue describing the defect, deficiency or desired
feature. You can read more about our issue and PR processes in the
wiki.

Updated CLI documentation to replicate serve information.

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.65%. Comparing base (403fdb1) to head (c7fd4ad).
Report is 41 commits behind head on branch-3.5.

Additional details and impacted files
@@             Coverage Diff             @@
##           branch-3.5   #13774   +/-   ##
===========================================
  Coverage       92.65%   92.65%           
===========================================
  Files             326      326           
  Lines           20733    20733           
===========================================
  Hits            19210    19210           
  Misses           1523     1523           

@bryevdv
Copy link
Member

bryevdv commented Mar 20, 2024

Hi @xmaniic thanks for the PR, FYI just duplicating the refguide section here results in errors:

/usr/share/miniconda3/envs/bk-test/lib/python3.10/site-packages/bokeh/command/subcommands/serve.py:docstring of bokeh.command.subcommands.serve.Serve.name:1: WARNING: duplicate object description of bokeh.command.subcommands.serve.Serve.name, other instance in docs/reference/command/subcommands/serve, use :no-index: for one of them

As suggested you could add :no-index: to the top of this page to prevent the warnings.

Alternatively, we can move almost all the ReST content out of the serve.py module, and into cli.rst insted. Arguable, the reference guide for bokeh.commands.subcommands.serve should really cover the API usage and not the CLI usage.

I 'd actually suggest trying the second approach (moving everthing to cli.rst) but I will accept the current PR with :no-index added if you don't have more bandwidth.

@bryevdv
Copy link
Member

bryevdv commented Mar 20, 2024

Also, FYI I think the linked issue at the top is incorrect (this link is to this PR itself)

@xmaniic
Copy link
Author

xmaniic commented Mar 20, 2024

I added :no-index: to my pull request message, is that the spot you were referring to?

And yes, I can go ahead and move the REST info over to cli.rst.

@bryevdv
Copy link
Member

bryevdv commented Mar 20, 2024

I added :no-index: to my pull request message, is that the spot you were referring to?

No the :no-index: would be added to cli.rst however if you are going to move the ReST content out of serve.py and into cli.rst then it will not be necessary.

@mattpap mattpap added this to the 3.5 milestone Mar 20, 2024
@bryevdv
Copy link
Member

bryevdv commented Apr 7, 2024

@xmaniic did you intend to continue working on the PR?

@bryevdv
Copy link
Member

bryevdv commented Apr 22, 2024

@xmaniic checking one more time, do you expect to continue on this PR? If not that's fine but should be closed.

@bryevdv bryevdv removed this from the 3.5 milestone May 5, 2024
@bryevdv
Copy link
Member

bryevdv commented May 5, 2024

Closing for lack of response.

@bryevdv bryevdv closed this May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Users Guide sections for server CLI is not useful
3 participants