Rename `IComponent` result types by danth · Pull Request #797 · eclipse-score/lifecycle

danth · 2026-10-15T11:14:51Z

I struggled to understand the original code because some values had multiple meanings, and different things were valid depending on the context.

Split into two results, one for de/activate and one for termination. The interesting changes are in icomponent.hpp.

Part of addressing #705 (comment)

WilliamRoebuck

I like removing the error before/after ready distinction, that part was always quite confusing to me. Unfortunately though, a lot of the new changes have just become out of date yesterday since #738.

In summary, the whole async issue is no longer present, kWaiting can probably be removed. As far as I know:

tryReportCompletion is no longer needed. Existing calls should be replaced or removed
tryHandleTermination - should now only flag an error to the caller or return some sort of void/ignore result (no error yet). Should never return kSuccess. Terminations during startup are resolved at the end using the stored termination result member.
activate & deactivate - always runs synchronously and returns success or failure

This all probably allows process monitor to be simplified as well

There are also some leftover unit tests (several of the parameterized ones) that test various async termination scenarios, most of which can be removed.

danth · 2026-10-15T15:40:43Z

In summary, the whole async issue is no longer present

This is good to hear. I think most of the dead code will be replaced anyway as part of #705, so in this pull request I'll only make the minimal edits needed to change the error types.

danth · 2026-10-16T10:22:35Z

@WilliamRoebuck I've removed the async values from the enums. I'd appreciate if you could take another look :))

The old result was difficult to understand because some values had multiple meanings, and different things were valid depending on the context. Split into two results, one for de/activate and one for termination. The interesting changes are in `icomponent.hpp`.

I only made the minimal edits in `ProcessInfoNode` to be able to update the enums: there is still a lot of code which does not really make sense any more. However, the new `Component` class shall eventually replace `ProcessInfoNode`, so there is not much point refactoring it further.

github-actions · 2026-10-16T11:12:30Z

Documentation preview for this pull request is available at:
pr-797: https://eclipse-score.github.io/lifecycle/pr-797/

MaciejKaszynski · 2026-10-19T14:02:53Z

    {
-        run_target.activate(stop_source_.get_token());
+        const IComponent::RequestResult result = run_target.activate(stop_source_.get_token());
+        SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess);


Hmm I think it's right now safe to assert here because the run_target.activate just sets variable but not sure if we should assume that will be the case forever. I would rather log an error I think?

Originally we just ignored the return value entirely. I think logging an error could have the same consequences (errors going unnoticed) - the log message might not be noticed if tests are passing.

I would prefer keeping the assert and adding proper error handling if/when the need arises

I think the assert is good here because other changes might be needed if this did change, needs an assert message though to clear things up (e.g. "Run target activation cannot fail").

Also, is SCORE_LANGUAGE_FUTURECPP_ASSERT debug or production? Probably doesn't matter here but this seems ambiguous. Or did the API change?

Also, is SCORE_LANGUAGE_FUTURECPP_ASSERT debug or production?

Neither...

Compilation flag

Default Debug Production

Assertion type
Default Yes Yes No

Debug No Yes No

Production Yes Yes Yes

WilliamRoebuck

Looks great now, I only have two small suggestions

WilliamRoebuck · 2026-10-20T10:01:36Z

    IdentifierHash node_identifier;
-    /// @brief Description of the error.
-    IComponent::ComponentError reason;
+    /// @brief Why the activation failed, either kFailure or kTimeout.


I think RequestResult should still be an expected (expected_blank I think) with an error code. Something that can be kSuccess being an error code seems wrong to me.

WilliamRoebuck · 2026-10-20T10:09:42Z

    {
-        run_target.activate(stop_source_.get_token());
+        const IComponent::RequestResult result = run_target.activate(stop_source_.get_token());
+        SCORE_LANGUAGE_FUTURECPP_ASSERT(result == IComponent::RequestResult::kSuccess);


I think the assert is good here because other changes might be needed if this did change, needs an assert message though to clear things up (e.g. "Run target activation cannot fail").

Also, is SCORE_LANGUAGE_FUTURECPP_ASSERT debug or production? Probably doesn't matter here but this seems ambiguous. Or did the API change?

danth requested review from FScholPer, MaciejKaszynski, NicolasFussberger, pawelrutkaq and ramceb as code owners October 15, 2026 11:14

github-project-automation Bot added this to LCM - Lifecycle & Health FT Oct 15, 2026

danth requested review from anmittag and antonkri as code owners October 15, 2026 11:14

github-project-automation Bot moved this to Backlog in LCM - Lifecycle & Health FT Oct 15, 2026

danth had a problem deploying to workflow-approval October 15, 2026 11:14 — with GitHub Actions Error

danth had a problem deploying to workflow-approval October 15, 2026 11:25 — with GitHub Actions Error

WilliamRoebuck suggested changes Oct 15, 2026

View reviewed changes

danth had a problem deploying to workflow-approval October 16, 2026 10:20 — with GitHub Actions Error

danth requested a review from WilliamRoebuck October 16, 2026 10:22

danth added 3 commits October 16, 2026 11:58

Mark request result [[nodiscard]]

d3b1c12

danth force-pushed the pin-rename-result branch from 90a525a to 3d86568 Compare October 16, 2026 10:59

danth had a problem deploying to workflow-approval October 16, 2026 10:59 — with GitHub Actions Error

danth mentioned this pull request Oct 16, 2026

Introduce new Component class #705

Open

MaciejKaszynski reviewed Oct 19, 2026

View reviewed changes

Address review comments

0cf36e0

danth had a problem deploying to workflow-approval October 19, 2026 15:36 — with GitHub Actions Error

danth requested a review from MaciejKaszynski October 19, 2026 15:39

Merge latest main

59550a0

danth requested a deployment to workflow-approval October 20, 2026 09:19 — with GitHub Actions Waiting

WilliamRoebuck approved these changes Oct 20, 2026

View reviewed changes

Navigation Menu

Repository navigation

Rename `IComponent` result types - #797

Rename `IComponent` result types#797
danth wants to merge 5 commits into
eclipse-score:mainfrom
etas-contrib:pin-rename-result

danth commented Oct 15, 2026 •

edited

Loading

Uh oh!

WilliamRoebuck left a comment

Uh oh!

danth commented Oct 15, 2026

Uh oh!

danth commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

MaciejKaszynski Oct 19, 2026

Uh oh!

danth Oct 19, 2026

Uh oh!

WilliamRoebuck Oct 20, 2026

Uh oh!

danth Oct 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WilliamRoebuck left a comment

Uh oh!

WilliamRoebuck Oct 20, 2026

Uh oh!

WilliamRoebuck Oct 20, 2026

Uh oh!

This branch is waiting to be deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Compilation flag
		Default	Debug	Production
Assertion type	Default	Yes	Yes	No
	Debug	No	Yes	No
	Production	Yes	Yes	Yes

Conversation

danth commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WilliamRoebuck left a comment

Choose a reason for hiding this comment

Uh oh!

danth commented Oct 15, 2026

Uh oh!

danth commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

MaciejKaszynski Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

danth Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamRoebuck Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

danth Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WilliamRoebuck left a comment

Choose a reason for hiding this comment

Uh oh!

WilliamRoebuck Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

WilliamRoebuck Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

This branch is waiting to be deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danth commented Oct 15, 2026 •

edited

Loading