-
Notifications
You must be signed in to change notification settings - Fork 35
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
Draft: Final python2 version #861
Draft: Final python2 version #861
Conversation
…G_LIBRARY_STATES is true
as the docker build host is still too slow
* transitions and data-flows are now deleted automatically if "LIBRARY_RECOVERY_MODE" is set to true * if "LIBRARY_RECOVERY_MODE" is set to false you won't be able load broken state machines (DAU-securing-mechanism) * the default value of "LOAD_SM_WITH_CHECKS" is set to true; this means that consistency checks are now enabled per default; only set it to true if you are sure that no library interfaces changes were performed
to avoid segfault when setting the "gtk-theme-name"
There is no pressure from my side, I just want to ensure that I don't review a pull request that still receives commits :-) |
Ah, there is one question which I already had for the previews pull request: You mention "Support for custom designs", but I cannot find any code related to this. Is that really part of this PR? |
No, we will add it in the next pull request. I just deleted it. |
@franzlst Point 1, 2, and 4 of the #858 comment got resolved in this branch. However, I could not reproduce the 3rd point. I moved '99_bottles_of_beer' somewhere else and tried to open '99_bottles_of_beer_in_library'. After the replacement of the missing library with a dummy state, I moved '99_bottles_of_beer' back to its place and tried to substitute the dummy state with '99_bottles_of_beer', and everything looks fine. I guess the bug that you could somehow reproduce is related to 'substitute state' functionality and is not relevant to the better support of the missing libraries' feature. |
Thank you for the fixes! I now get a lot of
Steps to reproduce:
Not sure. If I try the same with a library (not a dummy state), substitution works:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you again for your fixes. In the code I added a hopefully last comment which you should consider fixing :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, so the pull request is ready from my side. Shell I merge it? Actually the honor is in your side @sebastian-brunner :-)
After merging, we also need a PyPI release, right?
@franzlst Our pipeline for python 2 was failing, so I went through the errors and fixed the issues. We had to back to 'imp' instead of 'importlib' which is python 2 friendly. Now, our pipeline is passing for python 2 & 3. |
Perfect! |
8452906
to
6e690c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good from my side. We discussed the changes internally already.
Ok thx for all the work @vahidm1993 and for your precious comments @franzlst . I merged the pr. Now the real show starts with releasing the first major. xD |
Could you create a release on pypi @franzlst ? The version would be 0.15.4., see the develop branch in which I updated the changelog again. |
Done so! I created the release on PyPI, as well as [here on GitHub]8https://github.com/DLR-RM/RAFCON/releases/tag/0.15.4). The master and develop branches should now both point to the latest commit. Also the rebuilt of the GitHub pages has been triggered. Thank you again for your contributions. I hope that I will find time next week for the next round of pull requests :-D |
Summary of pull request to merge in new features developed at Agile Robots AG.
Diff to the old pull request:
Open TODOs before the pr can be merged:
Open TODOs after the pr: