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.

Sign up for GitHub

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

Jump to bottom

NCU unstable namespace #12329

Merged
merged 1 commit into from
Nov 7, 2024
Merged

NCU unstable namespace #12329

merged 1 commit into from
Nov 7, 2024

Conversation

ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Oct 28, 2024 •
edited by skjnldsv
Loading

@ArtificialOwl ArtificialOwl mentioned this pull request Oct 28, 2024
Merged
provokateurin
provokateurin requested changes Oct 28, 2024
View reviewed changes
developer_manual/digging_deeper/api.rst Outdated Show resolved Hide resolved
developer_manual/digging_deeper/api.rst Outdated Show resolved Hide resolved
developer_manual/digging_deeper/api.rst Outdated Show resolved Hide resolved
developer_manual/digging_deeper/api.rst Outdated Show resolved Hide resolved
developer_manual/digging_deeper/api.rst Outdated Show resolved Hide resolved
@nickvergessen
Copy link
Member

  • OCP code must never mention NCT. It can not require it as an argument, constant or return something from NCT.
  • NCT will be added to the nextcloud-deps/OCP package for easier testing with psalm

@ArtificialOwl ArtificialOwl force-pushed the feat/48891/nct-namespace branch from 44ea69c to b5e9d6c Compare November 6, 2024 10:16
@ArtificialOwl
Copy link
Member Author

  • OCP code must never mention NCT. It can not require it as an argument, constant or return something from NCT.

    • NCT will be added to the nextcloud-deps/OCP package for easier testing with psalm

added to documentation

susnux
susnux reviewed Nov 6, 2024
View reviewed changes
developer_manual/digging_deeper/api.rst Outdated
@@ -7,3 +7,23 @@ PHP public API

The public API is contained in the OCP namespace. See the `OCP API reference
<https://nextcloud-server.netlify.app/>`_ for further details.


PHP testing API
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like this is about API for testing not API that is in testing state.
Maybe "Unstable PHP API"? As "unstable" is a more common term for this kind of API in general?

(At least for me "testing" is more like we provide our \Test namespace)

Copy link
Member

Choose a reason for hiding this comment

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

Was wondering similarly, also for the lib/testing/ folder in Server. Other options could be Experimental or Preview (colliding with previews/thumbnails)

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree on those points, but from previous vocal discussions:

  • testing is above unstable (at least from debian's point of view)
  • 'Experimental' would means NCE to be the name of the namespace, which is an already used acronym,
  • 'Preview' would also conflict,
  • 'devel' can be confusing too, as it can be interpreted as API for development ...

unstable might the only valid term that does not conflict that much and also reflect the 'unstable'/'prone to changes'

Copy link
Member Author

Choose a reason for hiding this comment

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

So, after discussion with @provokateurin, testing have been renamed to unstable.

Files location: /lib/unstable/
namespace: NCU\

@ArtificialOwl ArtificialOwl force-pushed the feat/48891/nct-namespace branch from ec1b2e3 to 21dcbe6 Compare November 7, 2024 18:58
@ArtificialOwl ArtificialOwl changed the title NCT testing namespace NCU unstable namespace Nov 7, 2024
@ArtificialOwl
NCU unstable namespace
7f1ed36 
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl ArtificialOwl force-pushed the feat/48891/nct-namespace branch from 21dcbe6 to 7f1ed36 Compare November 7, 2024 20:23
@ArtificialOwl
Copy link
Member Author

squashed, screenshot updated, merged.

@ArtificialOwl ArtificialOwl merged commit 633d170 into master Nov 7, 2024
12 checks passed
@ArtificialOwl ArtificialOwl deleted the feat/48891/nct-namespace branch November 7, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux left review comments

@nickvergessen nickvergessen nickvergessen left review comments

@provokateurin provokateurin provokateurin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ArtificialOwl @nickvergessen @susnux @provokateurin