-
Notifications
You must be signed in to change notification settings - Fork 56
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
Contribute changes back to original repo #47
Comments
Hey! First of all thank you so much for looking into my code! Really damn appreciate it!
I will try to help with anything needed if you're interested in this! if you wonder what was my intention to rethink & improve the whole infra from the ground: I forked it at the end of the summer because I got a side request to make the application load worlds in fully offline mode + p2p multiplayer, I didn't have much time for testing and I had to dramatically speed up development, and I instantly realized what slows me down and made everything tested. This was done just for developer productivity! Generally, if some other guy doesn't know how to work with that he shouldn't be afraid of it :)
There are no TS repos using standardjs and this repo has much much more powerful linting setup. Yes, I'm thinking out of PrismarineJS repos scope, sorry.
Anything bad? I enjoy fast & efficient installs.
I don't mind extracting it back to a dependency. Initially, I wanted to turn this repo into monorepo since p-viewer is a core of the whole client, most of the time I needed to fix something in p-viewer for the client so I can release the changes instantly, but I don't mind putting it back.
What do you want? Just remove the large piece of work working on the types system (remove all the types for no reason)? we can also can put types into js with less concise syntax but I don't see a reason to do this, on the other hand this is better than nothing and this is what bluemap repo did I'm not the only guy using all these in a public github repo, for example famous DefinitelyType is also using pnpm... Again, thank you so much for your time and for keeping your interest up! |
Ideally I would like to keep coherence with the rest of PrismarineJS, but if not, at minimum I would like to have internal consistency in this repo. So for example if we're using typescript use it for all files. If using react, let's switch fully to it. But I'll play with some of these tools and come back to you, that seems to be an easier path than arguing over it. |
Move from my dependencies (and see what is blocked):
|
about pnpm: we can remove this large lockfile. I'd be more than happy with the solution of keeping the repo without lockfile, but we will have to lock all github deps to specific commit hash instead of branch in package.json for reproducible builds and use this script if want to update all deps: https://github.com/zardoy/prismarine-web-client/blob/aea90db31e398682de6aca827c162253e9b4ba5f/scripts/updateGitPackages.mjs#L5, but I'd prefer keeping pnpm on the ci to ensure we have the fastest builds. Also I'd prefer keeping vercel, can live without these awesome preview deploys that stay forever :) |
I guess my dream is your nightmare 🤣 i have literally everything in this repo Update: won't be possible to merge. |
are you interested for fixes for some of these ?
I figure we might as well fix your fork, and when it becomes ok enough, we merge back to upstream.
What do you think?
The text was updated successfully, but these errors were encountered: