Skip to content

Implement integration tests for fields with set and notifier#463

Draft
sahithi-nukala wants to merge 1 commit into
eclipse-score:mainfrom
sahithi-nukala:sah_add_integration_test_for_fields
Draft

Implement integration tests for fields with set and notifier#463
sahithi-nukala wants to merge 1 commit into
eclipse-score:mainfrom
sahithi-nukala:sah_add_integration_test_for_fields

Conversation

@sahithi-nukala
Copy link
Copy Markdown
Contributor

No description provided.

@sahithi-nukala sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch 3 times, most recently from 4a9e897 to 8306eb3 Compare May 21, 2026 13:53
@sahithi-nukala sahithi-nukala force-pushed the sah_add_integration_test_for_fields branch from 8306eb3 to 38982b6 Compare May 21, 2026 13:57

// Note : at the moment the SkeletonField::Get implementation is not in the branch which means the skeleton and
// proxy side does not have same template parameters.
template <typename SampleType, bool EnableSet = false, bool EnableGet = false, bool EnableNotifier = false>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you reorder these?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please put this change in a separate commit with a good description of why you're changing traits / traits_test.

public:
using InterfaceTrait::Base::Base;

static constexpr bool kEnableSet{std::is_same_v<InterfaceTrait, SkeletonTrait>};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you add this?

load("@score_baselibs//score/language/safecpp:toolchain_features.bzl", "COMPILER_WARNING_FEATURES")
load("//quality/unit_testing:unit_testing.bzl", "cc_unit_test")

cc_library(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Moving these should be a separate commit. You can read my comment here to better understand how commits should look / why we split them up: #385 (comment)

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.

2 participants