search

serversideup / financial-freedom

🔥🔥🔥 An open source alternative to Mint, YNAB, and more. Stay on budget and build wealth without sacrificing your privacy.
https://serversideup.net/open-source/financial-freedom/
GNU General Public License v3.0
1.66k stars 139 forks source link

Laravel 10 and all dependencies updated and using vitejs #60

Closed jeremykenedy closed 10 months ago

anheru88 commented 1 year ago

This Pr didn't work for me, the css file don't be injected in the view, and I try to fix this, but the problem persist. @jeremykenedy

jeremykenedy commented 1 year ago

Remove the node modules folder, do npm install then npm run build

https://github.com/jeremykenedy/financial-freedom/blob/96b4d9032b0084d6e2515f82c3ad0dd3a3a4b2e2/package.json#L5

it might also be your environment. I'm using Laravel valet to develop and it works just fine

anheru88 commented 1 year ago

Please change in the next file: https://github.com/jeremykenedy/financial-freedom/blob/96b4d9032b0084d6e2515f82c3ad0dd3a3a4b2e2/resources/views/welcome.blade.php#L7 for: @vite('resources/css/app.css')

arukompas commented 10 months ago

From what I see, the PR contains a lot of different stuff:

There's so much stuff going on, making it a massive PR that can take time to review for the maintainers and decide whether to merge it or not.

And what if the Laravel 10 upgrade is welcome, but the change to Vite is not? That's why you need to apply Single Responsibility to PRs as well - change one thing at a time, whether it's Laravel 10 upgrade, or change to Vite, or feature tests added, etc.

This PR would be ideally split into smaller chunks, which themselves could be reviewed more easily and then merged, or not merged - individually.

Just my 2 cents.

willwhitelaw94 commented 10 months ago

Sorry have you upgraded it to Laravel 10 yet.

Do you plan to do it soon?

danpastori commented 10 months ago

@arukompas I agree. I really appreciate the hard work and PR, but I think we will have to do this in a little bit smaller, piece-meal fashion. We also have to upgrade InertiaJS to 1.0. I'll probably use this as a super helpful guide and make individual PRs.

@willwhitelaw94 We haven't got to L10 yet. That will be the first upgrade I make though.

jaydrogers commented 10 months ago

Hey all,

Just wanted to chime in that we're extremely grateful for the efforts on the PR here.

As we stated before, this was a side project that took off faster than we expected. This PR arrived in between the waiting time of wrapping up our other projects and re-vamping this project into a sustainable open source product.

Where we're at with the project now

We've been investing significant time in cleaning the project up. This includes making it easier to contribute, install, and onboard people.

Part of this process is overhauling how we have things structured to make it easier for other people to customize and contribute.

We started with the README file, which you can see has significant changes: https://github.com/serversideup/financial-freedom

Where this PR stands

Although we're very grateful for the PR, unfortunately I don't think it is able to be merged. We have so many changes coming down the pipeline, it would make most sense for our internal team to apply these updates and have a "solid concrete foundation" for everyone before we start taking on PRs.

There's just so much in motion, it would just be faster for us to go through and do this ourselves rather than getting acquainted with other code.

We're very grateful for your support and contribution @jeremykenedy! We're excited to get you a much better version of everything so we can continue to build this product together 👍