+
Skip to content

Conversation

jvidalor
Copy link
Contributor

@jvidalor jvidalor commented Jun 2, 2025

Adter the last server changes in v11, the PropertyField location is defined on its FieldDefinition and it is independent from the location of its Scoping. Changes are made to the setters and getters to support this.

@jvidalor jvidalor requested a review from a team as a code owner June 2, 2025 08:58
Copy link

codecov bot commented Jun 2, 2025

Codecov Report

❌ Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@dbd0284). Learn more about missing BASE report.
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2322   +/-   ##
=======================================
  Coverage        ?   84.07%           
=======================================
  Files           ?       91           
  Lines           ?    10701           
  Branches        ?        0           
=======================================
  Hits            ?     8997           
  Misses          ?     1704           
  Partials        ?        0           

@moe-ad moe-ad changed the base branch from master to main July 28, 2025 09:44
@rafacanton
Copy link
Contributor

sync successful CI

@rafacanton rafacanton changed the title jvidalor/locations in property field Support PropertyField location after server changes Jul 31, 2025
@jvidalor jvidalor force-pushed the jvidalor/locations_in_property_field branch from 3a5d304 to 454ae04 Compare August 1, 2025 09:06
)
self._field_definition_instance = None
if meets_version(self._server.version, "11.0"):
self._field_definition_instance = self._load_field_definition()
Copy link
Contributor

Choose a reason for hiding this comment

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

why doing it at init and not lazy anymore? It can have non negligeable impact on performance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, let's see if the tests are not impacted by this change. I was replicating the behavior of the Field, but you are right on the performance point.

return field_grpcapi._get_stub(client).Create(request)

@staticmethod
def csproperty_field_new_location_on_client(client, numEntities, data_size, location):
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we need a new API here, couldn't we do field_def_new and then set_location?

@jvidalor jvidalor merged commit a175c71 into main Aug 5, 2025
47 checks passed
@jvidalor jvidalor deleted the jvidalor/locations_in_property_field branch August 5, 2025 15:02
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.

4 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载