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

Add language specification to property names #2795

Merged
merged 6 commits into from
Sep 26, 2024

Conversation

externl
Copy link
Member

@externl externl commented Sep 26, 2024

This is the first step in implementing #2746. Namely,

Properties in PropertyNames.xml now require an additional attribute languages="...". This specifies the languages which support this property. Languages which do not support the property do not generate an entry for it. As a consequence they'll warnings will be issued they you try to use an unsupported property.

languages is a comma separated list of cpp, csharp, java, js, or a all (by itself).

Reworked makeprops to delay generation until after we've parsed all the properties. We need to know ahead of time if an entire section will be empty or not.

@externl externl added this to the 3.8.0 milestone Sep 26, 2024
<suffix name="MessageSizeMax" />
<suffix name="AdapterId" languages="cpp,csharp,java" />
<suffix name="Connection" class="connection" languages="cpp,csharp,java"/>
<suffix name="Endpoints" languages="all" />
Copy link
Member

Choose a reason for hiding this comment

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

TODO: no Endpoints in JS

config/PropertyNames.xml Show resolved Hide resolved
<property name="BackgroundLocatorCacheUpdates" languages="all" default="0" />
<property name="BatchAutoFlush" deprecated="true" languages="all" />
<property name="BatchAutoFlushSize" default="1024" languages="all" />
<property name="ChangeUser" languages="cpp" />
Copy link
Member

Choose a reason for hiding this comment

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

We should consider removing Ice.ChangeUser.

<property name="SyslogFacility" languages="cpp,java" default="LOG_USER" />
<property name="ThreadPool.Client" class="threadpool" languages="cpp,csharp,java" />
<property name="ThreadPool.Server" class="threadpool" languages="cpp,csharp,java" />
<property name="ThreadPriority" languages="cpp,csharp,java" />
Copy link
Member

Choose a reason for hiding this comment

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

Remove from cpp?

config/PropertyNames.xml Outdated Show resolved Hide resolved
<property name="Discovery.Locator" class="objectadapter" />
<property name="Trace.Observers" />
<property name="Trace.SaveToRegistry" />
<property name="AuthenticateUsingSSL" languages="cpp,java" />
Copy link
Member

Choose a reason for hiding this comment

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

I suspect some properties are cpp-only or java-only, e.g. Trace.SaveToRegistry is java-only.

config/PropertyNames.xml Outdated Show resolved Hide resolved
</section>

<section name="IceBoxAdmin">
<property name="ServiceManager.Proxy" class="proxy"/>
<property name="ServiceManager.Proxy" class="proxy" languages="cpp" />
Copy link
Member

Choose a reason for hiding this comment

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

We actually have IceBoxAdmin in java too.

<property name="Replica" languages="cpp,java" />
<property name="Host" languages="cpp,java" />
<property name="Port" languages="cpp,java" />
<property name="InstanceName" default="IceGridAdmin" languages="cpp,java" />
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 a strange property with a strange default.

<property name="Compression.Level" languages="cpp,csharp,java" default="1" />
<property name="Config" languages="cpp,csharp,java" />
<property name="Connection.Client" class="connection" languages="all" />
<property name="Connection.Server" class="connection" languages="all" />
Copy link
Member

Choose a reason for hiding this comment

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

No server connections in JavaScript

@externl externl merged commit 4bdf7c3 into zeroc-ice:main Sep 26, 2024
17 of 18 checks passed
@externl externl deleted the 2746-better-property-names branch September 26, 2024 18:52
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.

3 participants