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

Upgrade to Spring Boot v3 #26

Open
firasrg opened this issue Jun 10, 2023 · 37 comments
Open

Upgrade to Spring Boot v3 #26

firasrg opened this issue Jun 10, 2023 · 37 comments

Comments

@firasrg
Copy link

firasrg commented Jun 10, 2023

Problem

I'm unable to run the app locally, I see many errors ❌ The app is built with Spring Boot v1.4 and I'm with JDK 17?! It seems this project has not received updates for long time (Last commit on /src folder was 6 years ago!), this indicates that it has become outdated so it lacks compatibility with newer techs, patterns and best practices.

Solution

I suggest to do an upgrade (ideally to latest version Spring Boot v3).

FYI: The main Spring PetClinic project is actively maintained and regularly updated! It can serve as an excellent reference to guide the upgrade process here.

Please let me know if you have any questions or require further information to support this issue.

@firasrg
Copy link
Author

firasrg commented Jun 10, 2023

FYI, I've started working on this issue in a forked repo.

@michaelisvy
Copy link
Collaborator

hello, I agree that it will be good to do an upgrade.
I wanted to install it today and it took me a while to figure out that we need Java 8 to run it because of the javax.xml.Binding packages incompatibility.

@firasrg I can see that you have started working on it. Happy to help if needed.

@arey
Copy link
Member

arey commented Jun 11, 2023

Hi @firasrg Thanks for your proposition. We are looking contributors to upgrade all the forks of the Petclinic Community and you are welcome. I think you can indeed draw inspiration from the main repository. I let you submit a Pull Request when you are ready.

@nilshartmann
Copy link
Collaborator

Thank you, @firasrg for your work!

@ALL: I wonder if we should follow the spring-petclinic-angular example and drop the backend part here and only include the react frontend.

Pro:

  • we can focus on implementing the React frontend (I don't think in the backend should be any React-specific stuff)
  • people can compare angular/react solutions without having to worry about the backend

Con:

  • we need to make sure that we're compatible with spring-petclinic-rest. (I think this is not really a problem as we can pin a commit or tag here).

What do you think?

@firasrg
Copy link
Author

firasrg commented Jun 11, 2023

@nilshartmann Hmm if there is already a reliable backend app, why repeating it here ? In that case I sugges to make this repo only for frontend app.

Ultimately, if absolutely you need backend and frontend source files together, then you can use Git Submodules feature, but that will require a new repo (I think).

So project hierarchy becomes like this

  • Spring Pet Clinic (main repo)
    • React App (repo frontend)
    • Rest API App (repo backend)

Otherwise I agree to make it fully frontend like the one for Angular.

@nilshartmann
Copy link
Collaborator

@firasrg: Git Submodules sounds like a good idea.

I think we don't need a new repo:

  • Spring Pet Clinic (this repo)
    • frontend/ (this repo)
    • REST API app (repo backend via submodule)

@firasrg
Copy link
Author

firasrg commented Jun 11, 2023

Hmm, that would be nice! So are you (collaborators/maintainers) going to do this radical change or contributors?

@arey
Copy link
Member

arey commented Jun 12, 2023

Hello Nils and Firas. If we could use the existing spring-petclinic-rest as backend it coud be great. The spring-petclinic-reactive demonstrates how to use a git submodule.

@nilshartmann
Copy link
Collaborator

Hi all,

again thanks a lot for your comments and engagement! 👍 😊

I would suggest:

  • we're using only one issue to track progress for backend and frontend, as both "parts" are connected
  • we can create a new branch for the work
    • when the work is done, we can also rename master to main
  • We have to somehow split the work so that @michaelisvy and @firasrg can work together in parallel (and whoever else want to join)
    • I currently don't have that much time but I'm more than willing to help if you have problems, questions or need any kind of review
    • As an "inspiration" for the React client, you could also have a look at my GraphQL React example. Not sure if that helps much, because it's build using create-react-app and also of course with GraphQL API, but maybe you can "steal" some components or ideas :-)

What do you think?

@firasrg
Copy link
Author

firasrg commented Jun 12, 2023

Hi again @nilshartmann And thank you for this feedback!

The other issue is closed🚪.

I think that tasks like rename main branch and adding submodules can be done by someone having authorities unlike me or any ordinary contributors.

As for the upgrade I think i can do i it effectively!

@michaelisvy I don't think that splitting the upgrade task is a good idea as the current version looks simple. Maybe we can add a 1st new feature, like a user authentication.

What do you think?

@firasrg
Copy link
Author

firasrg commented Jun 12, 2023

I have another interesting suggestion (it can be in another issue if you like it).

Make the client app as a React Native project, this can bring more interested people to the field 🌝.

As React Native gives possibility to build native apps (mobile/tablet) for Android/iOS in addition to Web with one codebase!

there is a nice framework called Expo that takes care of all native files. Im familiar with Expo and use it frequently to develop React Native. With Expo you only use JS/TS and Json files, no gradle files, no Xml, no swift, no Java. Expo is similar to Vite and CRA, it opens the road to work quickly and focus on logic rather than configurations.

Mobile apps now are trending.. many customers today, search to build more than one product with 1 codebase (with less efforts qnd time)

What do you think?

@nilshartmann
Copy link
Collaborator

Hi @firasrg,

while I think this is a good idea, I think that would make another/different spring-petclinic example project.

I'd like to go here with React, so people can compare with the angular approach, can get an idea how a React codebase would look like and so on.

@arey
Copy link
Member

arey commented Jun 13, 2023

Hi @firasrg

We don't yet have a mobile version of Petclinic dedicated to smartphones. The Kotlin version is a classic webapp, not an Android application.
Having a fork of Petclinic as a native application is a great idea. Why not with React Native? But like Niels, I'd be in favor of creating another repository. Let's perhaps concentrate first on upgrading this repo.

@michaelisvy
Copy link
Collaborator

hello,
I started something here: https://github.com/michaelisvy/petclinic-react
It's my first React application, so it's not great and still work in progress.

@firasrg
Copy link
Author

firasrg commented Jun 13, 2023

Hello everyone!

I did some good refactoring (still in progress). You can review it here 👈 .

Tbh, it's not like I expected at the beginning, it needs some hard work, as long as we think quality and long term maintenance.

@michaelisvy, thank you for this feedback, would you like to take a look at my version ? 😁

I'll keep you updated, see you

@michaelisvy
Copy link
Collaborator

yes it definitely takes a while. I'm happy to contribute to your repo if you think we can share the load.
I saw that you use Bootstrap. Is it more popular than Material-UI?

@firasrg
Copy link
Author

firasrg commented Jun 14, 2023

Im using bootstrap because it's mandatory to render styles correctly like in the old version.

I think ti go step by step: first I make it show up as expected then we see what to change.

Alright, Im going to add you as a collaborator

@michaelisvy
Copy link
Collaborator

thanks @firasrg ! I see you've added me as a contributor. Is there something I can do in priority?

@firasrg
Copy link
Author

firasrg commented Jun 16, 2023

You can open an issue and work on it 🤷🏻‍♂️

@michaelisvy
Copy link
Collaborator

ok I've sent you a pull request. Still a lot to do :)

@firasrg
Copy link
Author

firasrg commented Jun 17, 2023

@michaelisvy thank you for your help.

Btw it seems your PR is sent to the upstream rep not the forked repository, the one I created.

👉 I think it's too early to do that, as you said, there is lot todo. So i invite you to change the merge target to the main branch in the forked repo only. So i can review , merge or reject.

@michaelisvy
Copy link
Collaborator

ah indeed! I've closed the previous PR and created another one here: firasrg#4

@michaelisvy
Copy link
Collaborator

hello,
would it be possible to open our code base to more reviewers? For instance, @shonoru has reviewed my pull request but I don't think he has the right to approve it.

@firasrg
Copy link
Author

firasrg commented Jul 10, 2023

Hello @michaelisvy !

Thank you for your feedback and activities! I value all your efforts.

I think you can do merges by yourself as you are a member of that forked repo. I trust you!

Excuse me for being inactive during all this time, i have priorities..

Feel free to contact me, if you like, on my email, discord or any.. we can talk together and come to an idea

@michaelisvy
Copy link
Collaborator

ah is it? I thought you had to approve. I'll try to merge it.
Thanks for your feedback on my Pull Requests, I really learned from it :).

@michaelisvy
Copy link
Collaborator

michaelisvy commented Jul 10, 2023

actually, I don't seem to be able to merge.
I can see this message: "Merging is blocked
Merging can be performed automatically with 1 approving review."
Maybe it's a setting you can change?

@firasrg
Copy link
Author

firasrg commented Jul 10, 2023

Retry now please @michaelisvy

@shonoru
Copy link

shonoru commented Jul 10, 2023

don't seem to be able to merge

Please see, firasrg#8 (comment)

@firasrg
Copy link
Author

firasrg commented Jul 10, 2023

it seems the merge didn't work because you have a review comment still open! Remeber to resolve them before thinking to merge next time.

I've made merge rn, you can notice it.
Also, in order to get free from dependency on me, you can fork my repo, and continue your progress then do pull request. As for my repo, i still interested in it, just not finidin enough time.

Thanks all

@michaelisvy
Copy link
Collaborator

thanks @firasrg , I will fork your repository then.

@michaelisvy
Copy link
Collaborator

hi @firasrg @nilshartmann @shonoru , I was thinking to start again from the original project and migrate it little by little, instead of rewriting everything.
Unfortunately, package.json has so many libraries inside and some of them have changed name (such as @babel/core). Wondering if you have any tips, or if there is any tool to migrate an older project to a more recent version of libraries?

@nilshartmann
Copy link
Collaborator

Hi @michaelisvy!

I think it's a good idea to start from scratch. I don't think there is a clever migration strategy (tbh I forgot how old this example is 😱)

As create-react-app seems rather abandoned/deprecated now, I would start with a vite-based setup now. You could use the react-ts template.

Maybe it simplifies migration if you create a new folder parallel to client and start there. That way you can easily compare and copy+paste-things from the old client?

Please let me know I if can help you!

@shonoru
Copy link

shonoru commented Aug 1, 2023

I would start with a vite-based setup now

Would recommend the same actually, vite + react-ts bundle. In this case, you may re-use or copy existing components without much changes.

if you have any tips, or if there is any tool to migrate an older project

Nope, my understanding is that you're trying to update build tool rather than migrate components, it can be done either upgrade deps (could be painful), or replace it w vite as suggested above.

@michaelisvy
Copy link
Collaborator

hello, thanks a lot for the update!
Holidays are starting now, but I'll definitely go back to it in a few weeks :)

@nilshartmann
Copy link
Collaborator

@michaelisvy Holidays always are the best choice 🥳 Enjoy your free time!

@firasrg
Copy link
Author

firasrg commented Oct 20, 2023

hey everyone! i was absolutely absent on github for a while. I consider to continue working on this on my forked version. FYI

@michaelisvy
Copy link
Collaborator

hi @firasrg , that will be great! Sorry I was also focused on other things and didn't make improvements to my own fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants