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

Backporting TransportPlotting plugin to citadel #289

Closed
francocipollone opened this issue Sep 23, 2021 · 2 comments
Closed

Backporting TransportPlotting plugin to citadel #289

francocipollone opened this issue Sep 23, 2021 · 2 comments
Labels
🏰 citadel Ignition Citadel enhancement New feature or request

Comments

@francocipollone
Copy link
Contributor

francocipollone commented Sep 23, 2021

Context

The plotting feature was implemented for ign-gui4 (dome) and then forward-ported to newer releases, however, it was never backported to ign-gui3(citadel) which is a LTS.

Implementation

I tried backporting it to ign-gui3. Here is the commit/branch: 1a4d323

And it seems to be working correctly:

Screenshot from 2021-09-23 12-05-55

Note: I had started following the instructions as it is explained in the contributions section about what is the correct way to backport a feature, which is using cherry-pick and fixing the conflicts. However, this feature was added in several commits and given it was added a year ago, some changes/bug-fixes affected those files afterward so that method wasn't that smooth.

If you give me the thumbs up and there is nothing that prevents us from backporting it I will go ahead and create PR based on the branch I shared.

Related to #66

@chapulina
Copy link
Contributor

Thanks for looking into it! The problematic bit is this change:

-    class IGNITION_GUI_VISIBLE Application : public QGuiApplication
+    class IGNITION_GUI_VISIBLE Application : public QApplication

This breaks ABI and can't be done in a stable distribution.

@francocipollone
Copy link
Contributor Author

francocipollone commented Sep 23, 2021

Thanks for looking into it! The problematic bit is this change:

-    class IGNITION_GUI_VISIBLE Application : public QGuiApplication
+    class IGNITION_GUI_VISIBLE Application : public QApplication

This breaks ABI and can't be done in a stable distribution.

Thanks for the quick reply!
Oh you are right it can't be ported! Ok then, in our project we'll probably need to migrate to a newer release then.
(Fortress is candidate given that it is LTS, good timing :D).

Thanks again. Feel free to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants