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.

Sign up for GitHub

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

Jump to bottom

fix: Dev mode - 404 Page - Live reloaded list of static files #5217

Merged
merged 2 commits into from Nov 9, 2019
Merged

fix: Dev mode - 404 Page - Live reloaded list of static files #5217

merged 2 commits into from Nov 9, 2019

Conversation

machi1990
Copy link
Member

@machi1990 machi1990 commented Nov 5, 2019 •
edited

/cc @gsmet

Fixes #5201

@machi1990 machi1990 requested a review from gsmet November 5, 2019 13:47
aguibert
aguibert approved these changes Nov 5, 2019
View reviewed changes
Copy link
Member

@aguibert aguibert left a comment

Choose a reason for hiding this comment

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

LGTM

@machi1990
Copy link
Member Author

@gwenneg would also gave some time to review this? Related to improving the not found page in dev mode as #5235 thanks.

@gwenneg
Copy link
Member

gwenneg commented Nov 8, 2019

Sure, I'll take a look at it tonight.

gwenneg
gwenneg reviewed Nov 8, 2019
View reviewed changes
Copy link
Member

@gwenneg gwenneg left a comment

Choose a reason for hiding this comment

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

That's unrelated with your changes, but I've seen a few IOException being caught and rethrown encapsulated into a RuntimeException in RuntimeUpdatesProcessor. It would be nice to throw an UncheckedIOException instead while we're at it.

core/devmode/src/main/java/io/quarkus/dev/RuntimeUpdatesProcessor.java Show resolved Hide resolved
core/devmode/src/main/java/io/quarkus/dev/RuntimeUpdatesProcessor.java Show resolved Hide resolved
...teasy/deployment/src/test/java/io/quarkus/resteasy/test/NotFoundExceptionMapperTestCase.java Outdated Show resolved Hide resolved
@machi1990
Copy link
Member Author

That's unrelated with your changes, but I've seen a few IOException being caught and rethrown encapsulated into a RuntimeException in RuntimeUpdatesProcessor. It would be nice to throw an UncheckedIOException instead while we're at it.

Yes, let me do it in a separate refactoring commit.

@machi1990
Copy link
Member Author

@gwenneg thanks for the review. I pushed a second refactoring commit - WDYT?

gwenneg
gwenneg approved these changes Nov 8, 2019
View reviewed changes
Copy link
Member

@gwenneg gwenneg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@gwenneg gwenneg added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 8, 2019
@gwenneg gwenneg added this to the 1.1.0 milestone Nov 8, 2019
@gwenneg gwenneg merged commit 60be0d9 into quarkusio:master Nov 9, 2019
@machi1990 machi1990 deleted the fix/5201 branch November 9, 2019 08:29
@gsmet gsmet removed the backport? label Nov 14, 2019
@gsmet gsmet modified the milestones: 1.1.0, 1.0.0.Final Nov 14, 2019
@gsmet
Copy link
Member

gsmet commented Nov 14, 2019

In the end I decided to not backport it. It's not critical enough to justify touching the hot reload logic now.

@machi1990
Copy link
Member Author

Thanks @gsmet for the update. It will give me time (during the weekend) to look on why the test case added was failing in JDK 8, as pointed out by @dmlloyd here #5424.

@famod famod mentioned this pull request Jun 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aguibert aguibert aguibert approved these changes

@gwenneg gwenneg gwenneg approved these changes

@gsmet gsmet Awaiting requested review from gsmet

Assignees
No one assigned
Labels
area/devmode triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Milestone
1.1.0.CR1
Development

Successfully merging this pull request may close these issues.

Dev mode - 404 - List of static files not reloaded
4 participants
@machi1990 @gwenneg @gsmet @aguibert