Skip to content

docs: describe on_schedule as a Cloud Scheduler trigger - #321

Draft
IzaakGough wants to merge 1 commit into
mainfrom
docs/on-schedule-docstring
Draft

IzaakGough wants to merge 1 commit into
mainfrom
docs/on-schedule-docstring

Conversation

@IzaakGough

Copy link
Copy Markdown
Contributor

The on_schedule API reference said it handles Cloud Tasks and takes a CallableRequest. It actually runs on a Cloud Scheduler job and takes a ScheduledEvent.

This corrects that docstring, the broken ScheduledEvent link in its return type, and a typo in the ScheduledEvent docstring.

Tested by building the docs locally and running the docstring example. Not tested with the CI docs script.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request corrects typos and outdated descriptions in the docstrings of src/firebase_functions/scheduler_fn.py, ensuring that the on_schedule function and ScheduledEvent class are documented accurately. The reviewer recommends improving the Sphinx documentation syntax by replacing the exception role :exc: with :class: and :obj: for the ScheduledEvent class and None singleton respectively.

:type \\*\\*kwargs: as :exc:`firebase_functions.options.ScheduleOptions`
:rtype: :exc:`typing.Callable`
\\[ \\[ :exc:`firebase_functions.schedule_fn.ScheduledEvent` \\], :exc:`None` \\]
\\[ \\[ :exc:`firebase_functions.scheduler_fn.ScheduledEvent` \\], :exc:`None` \\]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Sphinx role :exc: is used to reference exceptions. Since ScheduledEvent is a class and None is a singleton object, they should be referenced using :class: and :obj: respectively instead of :exc:.

Suggested change
\\[ \\[ :exc:`firebase_functions.scheduler_fn.ScheduledEvent` \\], :exc:`None` \\]
\\[ \\[ :class:`firebase_functions.scheduler_fn.ScheduledEvent` \\], :obj:`None` \\]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant