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

Updated version #2

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

Updated version #2

wants to merge 10 commits into from

Conversation

Nitr0-G
Copy link

@Nitr0-G Nitr0-G commented May 9, 2023

Hello everyone.
This is a redesigned version of zydis-submodule-example, where its folder in "deps" is not needed, as well as itself. The .c file was updated for the new version of zydis, since the previous one was non-working, I also added a description for running under "Windows base OS"

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

Hey @Nitr0-G, thanks for updating the example!

However, I'm not entirely sure about the removal of the "Git" submodule.

While CMake does offer the ability to dynamically fetch the dependency these days, for some users this is not the preferred method.

Inexperienced users often fail when dealing with "Git" submodules. I therefore think that we need to look at "CMake" submodules and "Git" submodules separately here, and optimally show the use of both concepts.

Any opinion about this @athre0z ?

README.md Show resolved Hide resolved
src/README.txt Show resolved Hide resolved
src/myproject.c Show resolved Hide resolved
@Nitr0-G
Copy link
Author

Nitr0-G commented May 10, 2023

Hey @Nitr0-G, thanks for updating the example!

However, I'm not entirely sure about the removal of the "Git" submodule.

While CMake does offer the ability to dynamically fetch the dependency these days, for some users this is not the preferred method.

Inexperienced users often fail when dealing with "Git" submodules. I therefore think that we need to look at "CMake" submodules and "Git" submodules separately here, and optimally show the use of both concepts.

Any opinion about this @athre0z ?

Then I propose to demonstrate the use of both options in this example

@athre0z
Copy link
Member

athre0z commented May 18, 2023

Given that this repo is literally called "zydis-submodule-example", I feel that showing anything that isn't an actual example using a submodule might be rather misleading. Any repository with submodules will always have .gitmodules in the root, so even if we were to rename this repo to something else and created two directories for the both variants, it'd still be somewhat confusing. It we want to provide a FetchContent based example, perhaps it would be best to put it into a separate repository?

@Nitr0-G
Copy link
Author

Nitr0-G commented May 18, 2023

Given that this repo is literally called "zydis-submodule-example", I feel that showing anything that isn't an actual example using a submodule might be rather misleading. Any repository with submodules will always have .gitmodules in the root, so even if we were to rename this repo to something else and created two directories for the both variants, it'd still be somewhat confusing. It we want to provide a FetchContent based example, perhaps it would be best to put it into a separate repository?

So... Should I do this? I could do it. Let me know if I need to do this

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.

3 participants