Skip to content

Allow for the presence of both a default constructor and one which takes the properties for the service#1030

Merged
tcormackMW merged 6 commits intodevelopmentfrom
defaultConstruct
Aug 20, 2024
Merged

Allow for the presence of both a default constructor and one which takes the properties for the service#1030
tcormackMW merged 6 commits intodevelopmentfrom
defaultConstruct

Conversation

@tcormackMW
Copy link
Copy Markdown
Contributor

@tcormackMW tcormackMW commented Aug 8, 2024

Not allowing a user to write a default constructor for a DS service implementation class is not ideal.

You error, even it is is logically correct as a class.

If impl implements interface

class impl: public interface
    {
      public:
        impl(std::shared_ptr<cppmicroservices::AnyMap> properties);
        ~impl() override;
        std::string Description() override;

      private:
        std::shared_ptr<cppmicroservices::AnyMap> props;
    };

this is valid in DS.

But I cannot have:

class impl: public interface
    {
      public:
        impl(std::shared_ptr<cppmicroservices::AnyMap> properties);
        impl();
        ~impl() override;
        std::string Description() override;

      private:
        std::shared_ptr<cppmicroservices::AnyMap> props;
    };

The above will compile fail because it does not know whether to call the default or other constructor. In the case of both, it should call the one accepting properties, however a class should be allowed to have the default constructor always.

@tcormackMW tcormackMW self-assigned this Aug 8, 2024
github-actions[bot]
github-actions bot reviewed Aug 8, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread compendium/DeclarativeServices/test/gtest/TestComponentConfigurationImpl.cpp Outdated
Comment thread compendium/DeclarativeServices/test/gtest/TestComponentConfigurationImpl.cpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.cpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.cpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.cpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.hpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.hpp Outdated
Comment thread compendium/test_bundles/DSGraph08/src/ServiceImpl.hpp Outdated
Comment thread compendium/test_bundles/TestInterfaces/include/TestInterfaces/Interfaces.hpp Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 8, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.29%. Comparing base (aebd9f8) to head (a53f4fa).
Report is 2 commits behind head on development.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #1030      +/-   ##
===============================================
- Coverage        88.31%   88.29%   -0.02%     
===============================================
  Files              247      248       +1     
  Lines            11938    11931       -7     
===============================================
- Hits             10543    10535       -8     
- Misses            1395     1396       +1
Files Coverage Δ
.../servicecomponent/detail/ComponentInstanceImpl.hpp 81.30% <100.00%> (-2.70%) ⬇️

... and 6 files with indirect coverage changes

jeffdiclemente
jeffdiclemente requested changes Aug 9, 2024
View reviewed changes
Comment thread compendium/DeclarativeServices/test/gtest/TestComponentConfigurationImpl.cpp
github-actions[bot]
github-actions bot reviewed Aug 12, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
github-actions[bot]
github-actions bot reviewed Aug 12, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
jeffdiclemente
jeffdiclemente requested changes Aug 13, 2024
View reviewed changes
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
github-actions[bot]
github-actions bot reviewed Aug 13, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp
Comment thread compendium/ServiceComponent/test/TestComponentInstance.cpp Outdated
@tcormackMW tcormackMW merged commit 1a29618 into development Aug 20, 2024
@tcormackMW tcormackMW deleted the defaultConstruct branch August 20, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@jeffdiclemente jeffdiclemente jeffdiclemente approved these changes

@carneyweb carneyweb Awaiting requested review from carneyweb

Assignees

@tcormackMW tcormackMW

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tcormackMW @jeffdiclemente