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

refactor: fix: ! Split smallresources into subprojects #94

Merged
merged 15 commits into from
Apr 23, 2024
Merged

refactor: fix: ! Split smallresources into subprojects #94

merged 15 commits into from
Apr 23, 2024

Conversation

artur-michalak
Copy link
Contributor

@artur-michalak artur-michalak commented Apr 15, 2024

Description

Reading this issue I decided to do something.
In addition to the breakdown, I improved the scripts, including vehicles pushing, seat shuffe and teleports.

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My pull request fits the contribution guidelines & code conventions.

BREAKING CHANGE:
The project has been partially broken up.

@artur-michalak
Copy link
Contributor Author

artur-michalak commented Apr 15, 2024

outdated

  - action: download_github
    dest: ./resources/[qbx]/[qbx_smallresources]
    ref: main
    src: https://github.com/qbox-project/qbx_smallresources

@Manason
Copy link
Contributor

Manason commented Apr 16, 2024

Thanks for helping out with this. I like some of the splits like qbx_vehiclepush. Others, I feel are so trivial that it isn't worth the overhead of the team maintaining a resource. While I'm not sure what to do with them, if there is just a small snippet to basically call a native, I think it's fine for it to live in small resources for now, unless we have a really great split to put it in.

One option is to have a repository that is a collection of snippets, but isn't itself a resource. That would be a place where people can just upload snippets to do different things and server owners can copy and paste where they feel is best.

Another option is to keep small resources, but only include these small snippets, whereas anything of moderate size/complexity gets split out.

In any case, the strategy we've been using up until now has been to split when and where it makes sense and chip away at small resources. Maybe the smaller it gets the clearer the answer for what to do with the rest will become. That is to say, I think the best way forward with this PR is to agree on the most obvious splits, and do those first.

@artur-michalak
Copy link
Contributor Author

artur-michalak commented Apr 16, 2024

I agree with you. I made these splits to see what was worth tearing off. Keeping such scripts in smallresources separate modules and removal by server owners would also be good option.

@artur-michalak artur-michalak marked this pull request as ready for review April 22, 2024 21:07
@artur-michalak
Copy link
Contributor Author

I like such a breakdown, what do you think about it?

Copy link
Contributor

@Manason Manason left a comment

Choose a reason for hiding this comment

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

I'm not yet sure that all of these should be split into a separate repo/resource, but I think this is definitely an improvement so fine to merge this.

@Manason Manason merged commit bd44037 into Qbox-project:main Apr 23, 2024
2 checks passed
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