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

Allow Lists of Metadata #3478

Merged

Conversation

InsertCreativityHere
Copy link
Member

@InsertCreativityHere InsertCreativityHere commented Feb 3, 2025

This PR fixes #3459. We now support listing multiple metadata in these ways:

["foo", "bar"]
["foo"] ["bar"]
["foo", "bar"] ["baz"] // even this!

I kept the escaping/metadata tests as-is (ie. they're still using the comma syntax),
but updated everywhere else to use this 'new' syntax.
In my opinion, this 'new' syntax should be preferred going forward (as it's the syntax we use in IceRPC).

@InsertCreativityHere
Copy link
Member Author

Note for reviewers:
Grammar.cpp is generated code. Don't waste your time reviewing it.

@@ -279,15 +279,15 @@ interface NodeSession
["cpp:const"] idempotent NodeObserver* getObserver();

/// Ask the registry to load the servers on the node.
["amd", "cpp:const"] idempotent void loadServers();
["amd"] ["cpp:const"] idempotent void loadServers();
Copy link
Member

Choose a reason for hiding this comment

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

I assume you kept some instances (tests) for the "," syntax?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the escape tests all still use the old syntax, and so do the metadata-specific tests in cpp/test/Slice.
Everything else was updated.

@InsertCreativityHere InsertCreativityHere merged commit 8b722e6 into zeroc-ice:main Feb 3, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't apply 2 metadata to operation
3 participants