Skip to content

Conversation

mj-xmr
Copy link
Contributor

@mj-xmr mj-xmr commented Mar 12, 2022

Separated the (soon to be) testable library code from the GUI executable code and structured the dependencies better.

The project monero-wallet-gui now inherits a lot of settings from the library project walletqt, so they don't have to be repeated in the executable's project files.

@mj-xmr mj-xmr force-pushed the wallet-gui-has-own-directory branch from fce34ad to 0032cd0 Compare March 12, 2022 17:10
list(APPEND SOURCE_FILES "../qt/macoshelper.mm")
endif()

file(GLOB SOURCES_FOREIGN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally this will go to separate libraries, but we're not there yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have seen your comment here, but I think if we merge this as is this get even more complicated and we are just moving files around at some point.

@mj-xmr mj-xmr force-pushed the wallet-gui-has-own-directory branch from 0aea1f8 to 8bdf8f4 Compare March 12, 2022 20:24
@mj-xmr mj-xmr force-pushed the wallet-gui-has-own-directory branch from 8bdf8f4 to 323f225 Compare March 12, 2022 20:32
Copy link
Collaborator

@selsta selsta left a comment

Choose a reason for hiding this comment

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

libwalletqt must be a library that only that glues together libwallet with Qt. Everything else should not be part of it. See my other comment.

${Qt5Gui_PRIVATE_INCLUDE_DIRS}
)

target_link_libraries(${PROJECT}
Copy link
Collaborator

@selsta selsta Mar 17, 2022

Choose a reason for hiding this comment

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

This here should only link with wallet_api and ${QT5_LIBRARIES}. Everything else seems to be unrelated. Same with the unrelated foreign source files that get added.

@selsta
Copy link
Collaborator

selsta commented Mar 17, 2022

One more thing to consider, inside the model folder there are some that are related to libwallet and some aren't. I don't know how to split these up.

@mj-xmr
Copy link
Contributor Author

mj-xmr commented Mar 17, 2022

One more thing to consider, inside the model folder there are some that are related to libwallet and some aren't. I don't know how to split these up.

So I'm taking a closer look and it seems that it's all interconnected. Either I go all in and fix it all in one go, creating an unrealistically large PR, or we go step-by-step, leveraging "unlocked features" at each step. What's your choice?

@selsta
Copy link
Collaborator

selsta commented Mar 17, 2022

I don't know how you want to organize things so I can't really say at this point. Maybe you can make an overview?

@mj-xmr
Copy link
Contributor Author

mj-xmr commented Mar 19, 2022

I don't know how you want to organize things so I can't really say at this point. Maybe you can make an overview?

I'll get back to you on this one soon.

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

Successfully merging this pull request may close these issues.

2 participants