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

Duplicate code across examples #99

Closed
GitGhillie opened this issue Jan 15, 2024 · 6 comments · Fixed by #122
Closed

Duplicate code across examples #99

GitGhillie opened this issue Jan 15, 2024 · 6 comments · Fixed by #122

Comments

@GitGhillie
Copy link
Contributor

Looks like there is quite a lot of duplicate code across the examples ('core' folder for example).
This will make it painful if something in there needs to be changed (due to a new Bevy release for example).

If you want I'd be happy to try and factor these out.

@kaosat-dev
Copy link
Owner

You are spot on once again :) It has been bothering me too for a while.
Even more so as some aspects have been improved in some of the examples , and left in an earlier stage for others.
(not to mention other improvements I did inside external projects that could literally be copy pasted to improve some examples).

The one thing holding me back is the potential lack of clarity for new users: (this is really a question):

  • would it not be confusing to have examples with "local" dependencies ?
  • Or too much going through the specific code and then the common one ?

I would need to make a first pass at updating the "core" of at least one example to the latest & shiniest version, but would gladly accept your offer to do the rest of the refactoring after that if you feel up to it, much appreciated :)

@GitGhillie
Copy link
Contributor Author

would it not be confusing to have examples with "local" dependencies ?

It will be my first time doing a refactor like this with Rust but I imagine each example will have an import (or multiple) and if we place a comment at the import explaining what is going on then I think it will easy to understand.

Or too much going through the specific code and then the common one ?

Not sure if I understand what you mean here. If the duplicate code is factored out it will also be easier for users to distinguish between what the example is showing and what is kind of boilerplate etc.

Let me know if you've got a golden example!

@kaosat-dev
Copy link
Owner

would it not be confusing to have examples with "local" dependencies ?

It will be my first time doing a refactor like this with Rust but I imagine each example will have an import (or multiple) and if we place a comment at the import explaining what is going on then I think it will easy to understand.

Or too much going through the specific code and then the common one ?

Not sure if I understand what you mean here. If the duplicate code is factored out it will also be easier for users to distinguish between what the example is showing and what is kind of boilerplate etc.

Let me know if you've got a golden example!

Sorry I haven't responded yet, still churning on in the giant UI tool PR 😐 once that is released I will go back to this issue as well, the project needs a spring cleaning 😅

@GitGhillie
Copy link
Contributor Author

No problem! For me personally #102 is a bigger issue than cleanup anyways

@kaosat-dev
Copy link
Owner

@GitGhillie sorry to tag you afterwards, I just pushed a PR with most of the cleanup work done for examples , and to make Clippy happy .
It is a a raw first pass of cleaning up the examples, perhaps I should not have closed this issue just yet ?
Anyway, the basics are there , if you feel like doing more improvements in this area, please do :)

@GitGhillie
Copy link
Contributor Author

All good! I have some free time coming up so I will be happy to take a look then.
Also good to see the green checkmark finally 😆

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 a pull request may close this issue.

2 participants