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

[developer] Add info about the Circles app #10421

Closed
wants to merge 1 commit into from
Closed

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 22, 2023

☑️ Resolves

Add section about the Circles app

  • Some basic information on how to access the Circles API
  • Add a link to the API reference

🖼️ Screenshot

image

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good otherwise

The Circles app is installed by default since Nextcloud 22.
It can be used by other apps for creating user managed groups called `circles`, an example of such an apps is the `Collectives` app.

There is also the `Circles API reference <https://nextcloud.github.io/circles>`_ .
Copy link
Member

Choose a reason for hiding this comment

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

This is not a public API, is it? OCA structs should not be used by other apps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is at least the published parts (basically you use the CirclesManager). But I am not that into the internals of the circles app, at least that is how all apps I found use the circles API (e.g. collectives).

Copy link
Member

Choose a reason for hiding this comment

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

I think it's known that the Collectives app doesn't use a public API to access Circles features. But that's a risk the Circles maintainers take. We should not advertise the private API in our official dev docs. Structs in OCA have no stability guarantees.

If Circles has an API for other apps to consume it has to go into OCP.

To move forward I suggest not documenting the API. @juliushaertl @nickvergessen other ideas?

Copy link
Member

Choose a reason for hiding this comment

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

Would agree that we should only add API documentation to docs.nextcloud.com that is actually covered by what we consider a public and stable API. Maybe we can just move this to the circles app for now?

In the end we should actually have this as an OCP API instead of requiring using OCA classes.

@ArtificialOwl Iirc remember there was a ticket about that, but I can only find nextcloud/server#31341 (comment). I would be curious for what reason that was decided.

Copy link
Member

Choose a reason for hiding this comment

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

I asked for an official API back then already when we had to implement it in Talk:
https://github.com/search?q=repo%3Anextcloud%2Fspreed%20OCA%5CCircles&type=code

So yeah, currently state of the art, but should not be in the official docs since it does not follow the 3 years deprecation cycle at the moment.

* Some basic information on how to access the Circles API
* Add a link to the API reference

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux force-pushed the feat/dev-circles branch from 0b8b706 to 21e05ec Compare May 22, 2023 13:37
@rakekniven
Copy link
Member

Will close it after reading the comments.

@rakekniven rakekniven closed this Jun 7, 2023
@nickvergessen nickvergessen deleted the feat/dev-circles branch June 21, 2023 14:37
@marcelklehr
Copy link
Member

So, currently, there are no docs at all. That sucks.

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.

6 participants