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 camunda REST API doc #4196

Closed
wants to merge 1 commit into from
Closed

Update camunda REST API doc #4196

wants to merge 1 commit into from

Conversation

github-actions[bot]
Copy link
Contributor

This PR updates the camunda REST API

@nicpuppa
Copy link
Contributor

nicpuppa commented Aug 26, 2024

Hey @akeller @pepopowitz @tmetzke,

This is a PR generated by the new rest docs sync workflow. The workflow PR is still a draft but I think it's ready to be reviewed (small adjustments to add).
Please let me know your thoughts 👍

@akeller akeller added dx Documentation infrastructure typically handled by the Camunda DX team deploy Stand up a temporary docs site with this PR labels Aug 26, 2024
@github-actions github-actions bot temporarily deployed to camunda-docs August 26, 2024 18:29 Destroyed
Copy link
Contributor Author

The preview environment relating to the commit 1dcaccf has successfully been deployed. You can access it at https://preview.docs.camunda.cloud/pr-4196/index.html

Copy link
Collaborator

@pepopowitz pepopowitz left a comment

Choose a reason for hiding this comment

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

I love that you automated this!! I'm excited for #4168 ❤️ ❤️ ❤️ ❤️

Changes requested

There are many properties missing descriptions. This is something we should resolve -- my preference would be that the authors of the API update the upstream source first, then propagate them here....but if we need to add descriptions here first, and then open a separate PR to port them upstream, that is fine too. Either way, please let me know if my assistance is needed for any of it.

No changes requested, but noteworthy

There are regressions in many descriptions based on previous reviews, but I'm not very worried about those. Especially knowing that ensuing PRs would fix them, as long as we're pushing them upstream (example: camunda/camunda#21596).

Comment on lines +1303 to +1320
type: object
properties:
ownerKey:
type: string
ownerType:
type: string
enum:
- USER
- ROLE
- GROUP
resourceKey:
type: string
resourceType:
type: string
permissions:
type: array
items:
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

These properties would benefit from descriptions:

image

Comment on lines +1343 to +1372
UserFilterRequest:
type: object
properties:
username:
type: "string"
name:
type: "string"
email:
type: "string"
UserResponse:
type: "object"
properties:
id:
type: "integer"
format: "int64"
username:
type: "string"
name:
type: "string"
email:
type: "string"
UserSearchResponse:
type: object
allOf:
- $ref: "#/components/schemas/SearchQueryResponse"
properties:
items:
type: array
items:
$ref: "#/components/schemas/UserResponse"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These properties are all missing descriptions.

image

@@ -41,8 +41,8 @@ paths:
get:
tags:
- License
summary: Get status of Camunda license
description: Obtains the status of the current Camunda license
summary: Get status of Camunda License
Copy link
Collaborator

Choose a reason for hiding this comment

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

This, and many changes like it, are a result of us not making changes upstream yet 😅

@christinaausley and I opened camunda/camunda#21596 today to fix that.

@christinaausley, I don't know how you feel, but I would be fine with these regressions making it into this PR knowing that they will be fixed by the next one. Otherwise, we'll want to manually fix these like in this PR like we did in #4166.

@nicpuppa
Copy link
Contributor

nicpuppa commented Aug 27, 2024

Hey @pepopowitz, thanks for the review.

This PR is just an example to show the result of the automation. I think it can be closed.
For the future, I'd suggest to review the source directly from the upstream, asking tech-writers for a review, wdyt ? (we can send a communication via slack if we decide this)
The automated PRs should just be checked quickly and merged afterward.

Happy to here other idea here 💪

@pepopowitz
Copy link
Collaborator

Hey @pepopowitz, thanks for the review.

This PR is just an example to show the result of the automation. I think it can be closed. For the future, I'd suggest to review the source directly from the upstream, asking tech-writers for a review, wdyt ? (we can send a communication via slack if we decide this) The automated PRs should just be checked quickly and merged afterward.

Happy to here other idea here 💪

  1. Oh, I didn't realize this PR was intended to be closed. In the future, can you call that out in the body of the PR?

  2. Yes, I strongly agree that we should do reviews of OpenAPI spec changes at the source, however that would probably fall on the @camunda/tech-writers so I defer to them in terms of their availability and interest. Or @akeller by delegation.

@nicpuppa
Copy link
Contributor

  1. Oh, I didn't realize this PR was intended to be closed. In the future, can you call that out in the body of the PR?

My bad, the body is generated by the workflow. I should convert the PR as draft to avoid the mistake 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Stand up a temporary docs site with this PR dx Documentation infrastructure typically handled by the Camunda DX team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants