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

[Modeler] Compatible Modeler project with last version #4673

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

BehnamBinesh
Copy link

Apply modification to build and use modeler in sofa framework


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

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

@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress refactoring Refactor code pr: fix Fix a bug labels Apr 23, 2024
@alxbilger
Copy link
Contributor

[ci-build][with-all-tests]

@BehnamBinesh
Copy link
Author

[Modeler] :: warning and print cleaned. deprecated function removed
[Modeler] :: Set modeler cmake ON


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

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

@BehnamBinesh
Copy link
Author

The work is done . now, testing is in progress . The label can be changed

@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Apr 29, 2024
@bakpaul
Copy link
Contributor

bakpaul commented Apr 29, 2024

[ci-build][with-all-tests]

@bakpaul
Copy link
Contributor

bakpaul commented Apr 30, 2024

You are clearly in a dev state, each time you push a commit it triggers a new build on the CI. I'll put the flag wip again until you tell me that you've finished your fixes to avoid loading the CI.

@bakpaul bakpaul added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Apr 30, 2024
@BehnamBinesh
Copy link
Author

You are clearly in a dev state, each time you push a commit it triggers a new build on the CI. I'll put the flag wip again until you tell me that you've finished your fixes to avoid loading the CI.

All changes was minor and i think I'm not in dev state now. sorry for loading on CI

@BehnamBinesh
Copy link
Author

All platform build successfully, I think it's better back to the review

@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels May 1, 2024
@bakpaul
Copy link
Contributor

bakpaul commented May 7, 2024

Hello !
I've read your PR and I am currently compiling it on my side to test the GUI and the Modeler.

Some changes will need to be performed in order to accept your PR :

  1. You have added some commented code that need to be removed.
  2. You have expended the namespaces of some files, which is not in accordance with our standard, it is certainly your IDE. But could you please revert those changes ?
  3. You have set default value to attributes directly in the class declaration, this should be done only in the constructor.
  4. You have made some breaking changes: for instance, you removed the call to the superinit of ConfigurationSetting. Do you have any justification for that ?

@bakpaul
Copy link
Contributor

bakpaul commented May 7, 2024

So I've compiled it, and it launches but crashes as soon as I add something to the graph. Du you have the same behavior ?

@hugtalbot hugtalbot changed the title [Modeler] : WIP : Compatible Modeler project with last version [Modeler] Compatible Modeler project with last version May 7, 2024
@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels May 7, 2024
@hugtalbot
Copy link
Contributor

Note the dev meeting @BehnamBinesh


This PR attests from the interest in having a Modeler around SOFA. This is a current concern among the core developers and we are working on the base bricks to build such a Modeler.
This PR should be cleaned (remove reverted changes from master commits) or it should be closed


@BehnamBinesh
Copy link
Author

BehnamBinesh commented May 8, 2024

@hugtalbot and @bakpaul
It's a good news that dev team working on base to build project like Modeler.
So in this step, Should I continue on Modeler project and fix the bugs and things that have been mentioned OR stop this PR completely Now?

@BehnamBinesh
Copy link
Author

BehnamBinesh commented May 8, 2024

@bakpaul
Can you tell me that what did you do to cause crash?
I can add component to scene and remove and apply link with save and load
I have only one crash in "FixedConstraint" component that I fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix Fix a bug pr: status wip Development in the pull-request is still in progress refactoring Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants