fix: merge enum values for same-named enums across endpoints#1362
Closed
premtsd-code wants to merge 2 commits intomasterfrom
Closed
fix: merge enum values for same-named enums across endpoints#1362premtsd-code wants to merge 2 commits intomasterfrom
premtsd-code wants to merge 2 commits intomasterfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Spec/Swagger2.php`:
- Around line 623-625: The assignment to $list[$enumName]['keys'] uses
array_unique without a mode, which collapses associative array entries because
PHP casts arrays to "Array" for string comparison; update the merge dedupe to
call array_unique(..., SORT_REGULAR) on the merged array of
$list[$enumName]['keys'] and $parameter['enumKeys'] (the line assigning
$list[$enumName]['keys']) so array elements are compared by content and enum key
metadata is preserved.
- Around line 620-625: The merge can receive null because $param['enumKeys']
(assigned from $parameter['x-enum-keys']) may be null; update the code so any
use of $parameter['enumKeys'] (and $param['enumKeys']) passed into array_merge
is defaulted to an empty array — e.g., when assigning and when merging into
$list[$enumName]['keys'] replace direct uses of $parameter['enumKeys'] with a
null-safe fallback (such as $parameter['enumKeys'] ?? []) so
array_merge($list[$enumName]['keys'], ...) never receives null; ensure the same
null-safe default is applied wherever $param['enumKeys'] might be merged or
initialized (references: $param['enumKeys'], $parameter['x-enum-keys'],
array_merge, $list[$enumName]['keys']).
In `@templates/cli/package.json.twig`:
- Line 49: The template currently references a non-published package version
("@appwrite.io/console": "^3.2.0"); update the dependency in
templates/cli/package.json.twig by changing the "@appwrite.io/console" version
from "^3.2.0" back to "^3.1.0" (or alternatively wait and ensure 3.2.0 is
published before merging) so generated CLIs will not fail during npm install.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@templates/cli/package.json.twig`:
- Line 49: Replace the mutable GitHub branch dependency for the console
SDK—specifically the entry "@appwrite.io/console":
"github:appwrite/sdk-for-console#dev"—with a published, pinned npm release (for
example "3.1.0" or a fixed semver like "^3.1.0") so installs are reproducible
and do not track the tip of the dev branch.
premtsd-code
force-pushed
the
fix-sdk-enum-merge
branch
2 times, most recently
from
4748b3c to
e4201ba
Compare
Contributor
Author
|
Closing: fix belongs in appwrite repo Format.php, not sdk-generator |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
in_array($enumName, $list)bug ingetRequestEnums()— it checked array values (which are always arrays) instead of keys (strings), so the check never matched and each endpoint silently overwrote the previous enum valuesResources), the generator now merges all unique values into a single unified enumResourcesenum to only contain values from the last migration endpoint (NHost), missing resource types likefunction,deployment,site,provider, etc.Related PR: appwrite/sdk-for-console#64
Summary by CodeRabbit