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: NuxtFramework #556

Closed
wants to merge 1 commit into from
Closed

Refactor: NuxtFramework #556

wants to merge 1 commit into from

Conversation

shivarm
Copy link
Member

@shivarm shivarm commented Oct 22, 2023

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • All new and existing tests passed.

@melloware melloware changed the title Refractor: NuxtFramework Refactor: NuxtFramework Oct 22, 2023
@melloware melloware requested a review from ia3andy October 22, 2023 11:58
@melloware
Copy link
Contributor

Title looks good, Nice PR.

@melloware melloware added the enhancement New feature or request label Oct 22, 2023
@melloware melloware added this to the 2.3.0 milestone Oct 22, 2023
Copy link
Collaborator

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

Coould you use the NextFramework class that you copied (instead of duplicating), and just edit the build dir, if it's really a different one.

@melloware did you check it works?

@melloware
Copy link
Contributor

I am not familiar with Nuxt but I assume its different than Next?

@ia3andy
Copy link
Collaborator

ia3andy commented Oct 23, 2023

I am not familiar with Nuxt but I assume its different than Next?

It's a different framework, but the logic for detection seems to be similar

@ia3andy
Copy link
Collaborator

ia3andy commented Oct 23, 2023

At least from the PR code, I am not familiar with it either.

@melloware
Copy link
Contributor

Oh i see what you are saying its so similar to Next should we refactor these two to be similar and just override the script its looking for?

@ia3andy
Copy link
Collaborator

ia3andy commented Oct 23, 2023

Oh i see what you are saying its so similar to Next should we refactor these two to be similar and just override the script its looking for?

yes and the build dir needs to be checked

@shivarm shivarm requested a review from ia3andy October 23, 2023 13:02
@shivarm shivarm marked this pull request as draft October 23, 2023 13:04
@shivarm shivarm marked this pull request as ready for review October 23, 2023 13:11
@ia3andy
Copy link
Collaborator

ia3andy commented Oct 23, 2023

From what you are saying, it looks to me that Nuxt is a regular (generic) framework. @melloware could you have a look?

@shivarm shivarm marked this pull request as draft October 23, 2023 13:50
@shivarm shivarm force-pushed the nuxt branch 2 times, most recently from c1e34a9 to 1a79ca6 Compare October 24, 2023 04:17
@shivarm shivarm marked this pull request as ready for review October 24, 2023 04:18
Refractor: NuxtFramework
@melloware
Copy link
Contributor

@shivam-sharma7 I don't understand what with Nuxt is NOT working with the current code?

NUXT(Set.of("nuxt dev"), generic("dist", "dev", 3000)),

@melloware
Copy link
Contributor

melloware commented Oct 25, 2023

@shivam-sharma7 Using the current 2.2.1 plugin Nuxt works out of the box.

Attached zip:
nuxt.zip

Unzip the project and Run mvn quarkus:dev and navigate to https://localhost:8080 its working fine?

@ia3andy
Copy link
Collaborator

ia3andy commented Oct 25, 2023

Something you can do though is print some Warning if no "build" is found on the GenericFramework.

@ia3andy ia3andy closed this Oct 25, 2023
@melloware melloware removed this from the 2.3.0 milestone Oct 25, 2023
@shivarm shivarm deleted the nuxt branch October 26, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants