Skip to content

Conversation

@provokateurin
Copy link
Member

#225

Otherwise {} from JSON will turn into [] in PHP and then into [] in JSON which is not valid.

@provokateurin provokateurin added bug Something isn't working 3. to review labels Mar 17, 2025
@provokateurin provokateurin force-pushed the fix/merge-specs/objects branch from dc0ad4c to e62c393 Compare March 17, 2025 07:52
@nickvergessen
Copy link
Member

Is this testable? :-X

@provokateurin
Copy link
Member Author

I'll open a draft PR in documentation, then you can modify it to use openapi-extractor from path and check the diff.

@provokateurin
Copy link
Member Author

You can now test it with nextcloud/documentation#12887.
The way I tested it was generating the merged spec on the current master branch, copying it somewhere else, checking out the branch, generating it again (you need to adjust the path to merge-specs.php first) and then diffing the new with the previous spec.

@provokateurin
Copy link
Member Author

But on this topic, I was thinking of maybe adding the merged spec to the server repo (in the root folder) and then we can also run it as part of the CI here to check for any diffs or failures. It wouldn't even need any adjustments in this repo, just in server.
This would also make the specifications more accessible, because for standalone apps they are in the root folder, but for server it's more complicated and less obvious for people who don't develop Nextcloud directly.

@nickvergessen
Copy link
Member

yeah that's what I meant

@provokateurin
Copy link
Member Author

nextcloud/server#51674

@provokateurin provokateurin force-pushed the fix/merge-specs/objects branch from e62c393 to 5ec759a Compare March 24, 2025 17:03
@provokateurin provokateurin force-pushed the fix/merge-specs/objects branch from 5ec759a to b71a6e5 Compare March 24, 2025 17:09
@provokateurin
Copy link
Member Author

@provokateurin
Copy link
Member Author

@nickvergessen can you check again, so we can merge the PR and close the topic?

@nickvergessen
Copy link
Member

I don't have time at the moment, so just merge it I guess?

@provokateurin provokateurin merged commit 4fcdc49 into main Apr 8, 2025
15 of 19 checks passed
@provokateurin provokateurin deleted the fix/merge-specs/objects branch April 8, 2025 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants