Skip to content

v0.28 tasks #1746

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

Merged
merged 17 commits into from
Jun 29, 2022
Merged

v0.28 tasks #1746

merged 17 commits into from
Jun 29, 2022

Conversation

dichotommy
Copy link
Contributor

@dichotommy dichotommy commented Jun 20, 2022

Changes related to the tasks API in v0.28. For a description of these changes, see the associated issue.

@dichotommy dichotommy changed the base branch from master to v0.28 June 20, 2022 16:57
Also delete code sample related to removed get one task by index endpoint.
@@ -6,99 +6,98 @@ The `/tasks` route gives information about the progress of [asynchronous operati
The task `uid` is incremented **globally.**
:::

## Get all tasks
## Get tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be a good idea if we set the example headings to H4 and add sidebarDepth: 2?That way the filtering and paginating headings will be more visible

Screen Shot 2022-06-21 at 14 48 06

Copy link
Contributor Author

@dichotommy dichotommy Jun 21, 2022

Choose a reason for hiding this comment

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

I don't think we should do it as part of this PR because it would make it less consistent w/ the other API ref pages where the example is always an H3. But in general, I think this is a great idea.

Rationale: we don't really need examples to show up in the sidebar, but we would like people to be able to easily find other things that aren't endpoints (e.g. object definitions, special subsections like filtering or pagination above). So it would make sense to use H3s for these, and H4s for all example + query parameter + body subsections. Then set sidebarDepth to 2 for the whole API reference section.

Could you make a note about this on the API reference template PR or issue, please?

Copy link
Member

Choose a reason for hiding this comment

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

I also like the idea of highlighting subsections that might help users, and I also agree it doesn't belong to this PR.

Something to take into account, though: HTML headings (h1 through h6) are semantic structures. As in, using an H4 after an H3 implies the H4 is a subsection of H3. I also think that not using headings sequentially (e.g. H1 followed by H3 instead of H1 followed by H2) can mess up the document outline.

What I'm trying to say is: we should avoid as much as possible using a specific heading only because of the effect this will have on the sidebar. AFAIK, this is particularly relevant when it comes to accessibility because screen readers rely on the document outline to help visually impaired users navigate a page. If we find ourselves willingly using tags erroneously, it's an indication we need to update our website/tooling/etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's very interesting Gui, thanks for bringing it up. Not that this is particularly relevant, but we already do a lot of this: using non-sequential heading levels not for their effect on the sidebar, but for their visual effect on the page. See: most "query parameters" or "body" sections in the API references (H4s belonging to parent H2s).

@dichotommy dichotommy marked this pull request as ready for review June 21, 2022 13:35
@guimachiavelli guimachiavelli linked an issue Jun 22, 2022 that may be closed by this pull request
@guimachiavelli guimachiavelli added this to the v0.28 milestone Jun 22, 2022
Copy link
Member

@guimachiavelli guimachiavelli left a comment

Choose a reason for hiding this comment

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

Added a few comments to the files themselves. Mostly minor stuff and questions.

Also, I noticed that most of the new code samples on /reference/api/tasks.md are in code blocks instead of the code sample component. That's for ease of review, correct?

Finally, I would encourage requesting a review from a core team member.


<RouteHighlighter method="GET" route="/tasks"/>

List all tasks globally, regardless of index. The `task` objects are contained in the `results` array.

Tasks are always returned in descending order of `uid`. This means that by default, **the most recently created `task` objects appear first**.
Copy link
Member

Choose a reason for hiding this comment

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

Should this be uid or taskUid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be uid. taskUid is only used in the summarized task object returned from async endpoints. Confusing, I know 🙁

dichotommy and others added 2 commits June 23, 2022 18:52
Also rename get_all_tasks_by_index_1
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

🚀

@maryamsulemani97
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 29, 2022

Build succeeded:

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.

v0.28: Changes to Tasks API
3 participants