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

[NG] MOVE SofaComponent* to packages #620

Merged
merged 24 commits into from
May 7, 2018

Conversation

guparan
Copy link
Contributor

@guparan guparan commented Mar 29, 2018

Please gently welcome this first NG pull-request 😊

The base idea was to remove the SofaComponent* packages from modules and to put them in their own place instead.
Doing this, I realized that SofaComponentBase contains actual components (it's not only a basic package as it should be).
Thus, it permitted me to introduce the first NG module Sofa.Component.Utils and fill it with SofaComponentBase intruders (MakeAliasComponent, MakeDataAliasComponent, MessageHandlerComponent, InfoComponent).

NG architecture is based on the discussions in #543.
Transition headers are also provided (in deprecated_layout folder) to ensure old includes.

I am also working with @damienmarchal on a way to cleanly automatize this kind of NG refactoring with python: see https://github.com/guparan/sofa2ng

Feedback is more than welcome!


This PR:

  • builds with SUCCESS for all platforms on the CI.
  • does not generate new warnings.
  • does not generate new unit test failures.
  • does not generate new scene test failures.
  • does not break API compatibility.
  • is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

@guparan guparan added project: Sofa NG pr: status to review To notify reviewers to review this pull-request refactoring Refactor code pr: clean Cleaning the code labels Mar 29, 2018
@guparan guparan mentioned this pull request Mar 29, 2018
@guparan
Copy link
Contributor Author

guparan commented Apr 3, 2018

[ci-build][with-scene-tests]

set(SOURCE_FILES config/${PROJECT_NAME}.cpp)

list(APPEND HEADER_FILES
deprecated_layout/SofaComponentBase/MakeAliasComponent.h
Copy link
Contributor

@damienmarchal damienmarchal Apr 4, 2018

Choose a reason for hiding this comment

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

Please validate deprecated_layout principle

)

list(APPEND HEADER_FILES
src/sofa/component/utils/MakeAliasComponent.h
Copy link
Contributor

@damienmarchal damienmarchal Apr 4, 2018

Choose a reason for hiding this comment

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

Please validate src/namespace/component principle

@@ -1,6 +1,8 @@
cmake_minimum_required(VERSION 3.1)
project(SofaGuiQt)

find_package(Sofa.Component.Utils)
Copy link
Contributor

@damienmarchal damienmarchal Apr 4, 2018

Choose a reason for hiding this comment

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

Please give feedback on "." naming convention for Sofa.component.Utils

@@ -0,0 +1,48 @@
/******************************************************************************
Copy link
Contributor

@damienmarchal damienmarchal Apr 4, 2018

Choose a reason for hiding this comment

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

Please validate this new way to name config.h files

@guparan guparan changed the base branch from master to ng April 17, 2018 12:27
@guparan guparan added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Apr 18, 2018
@hugtalbot hugtalbot merged commit 61a8423 into sofa-framework:ng May 7, 2018
@guparan guparan added this to the v18.06 milestone Jun 18, 2018
@guparan guparan removed this from the v18.06 milestone Nov 5, 2018
@guparan guparan added this to the dropped milestone Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: clean Cleaning the code pr: status ready Approved a pull-request, ready to be squashed refactoring Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants