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

Major refactoring needed #118

Open
salco opened this issue Aug 8, 2023 · 6 comments
Open

Major refactoring needed #118

salco opened this issue Aug 8, 2023 · 6 comments

Comments

@salco
Copy link
Contributor

salco commented Aug 8, 2023

I've cloned this repo last week and I had to change many apps (all had same issue: type declaration inside a header).
it seems that they are all a copy paste of certain header and source.

My question is: @gabonator with the use of Cmake and the library can I try to refactor the project?
It could help adding new apps, upgrading gcc, automate some process, even add unit test.

@gabonator
Copy link
Owner

Feel free to do it. I am using cmake only for generating XCode project for emulated environment for some applications. I do not think it is suitable for building whole OS. Upgrading gcc doesn't give much sense, there are no concurrency related stuff used here, and also it would break the gabuino cloud based compilation service. Adding more apps is highly welcome.

Yes, I know about that header issue, I have just changed some declaration in library and forgot to fix it in some apps... I am right now busy with other stuff, so I let the community to do further development/fixes

@McGr3g0r
Copy link

McGr3g0r commented Aug 9, 2023

Hi,
I had to make many changes such as this to make the whole project compile:

--- a/system/apps/test70_webusb/source/rpc.h
+++ b/system/apps/test70_webusb/source/rpc.h
@@ -30,7 +30,7 @@ namespace RPC
     if (strcmp(command, "LCD::BufferBegin")==0)
       return (uint32_t)static_cast<void(*)(CRect const&)>(BIOS::LCD::BufferBegin);
     if (strcmp(command, "LCD::BufferWrite")==0)
-      return (uint32_t)static_cast<void(*)(unsigned short*, int)>(BIOS::LCD::BufferWrite);
+      return (uint32_t)static_cast<void(*)(const unsigned short*, int)>(BIOS::LCD::BufferWrite);
     if (strcmp(command, "LCD::Background")==0)
       return (uint32_t)static_cast<void(*)(CRect const&, uint32_t, uint32_t)>(GUI::Background);
     if (strcmp(command, "LCD::Bar")==0)

@gabonator
Copy link
Owner

Yes, that is exactly it, it should be on three different places I guess, feel free to open PR

@McGr3g0r
Copy link

McGr3g0r commented Aug 9, 2023

#120

@salco
Copy link
Contributor Author

salco commented Aug 13, 2023

yeah I will test that the change are not like mine but I've also changed the dockerfile

@salco
Copy link
Contributor Author

salco commented Aug 17, 2023

the dockerfile work fine but for the refactoring I can push it in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants