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

Update to phoenix 1.6 issue #61 [Part 2] #62

Merged
merged 4 commits into from
Oct 16, 2021

Conversation

nelsonic
Copy link
Member

Building on the work done by @ajmeese7 in #61 ("Part 1") to update to this tutorial/repo to Phoenix 1.6,
this PR:

  • removes the files we no longer need and updates the templates to use /assets/ directory.

Follows the instructions in Chris McCord's guide #55 (comment)
Aaron had already done most of the work to upgrade. 🙌
This tidy up was only required for anyone who had the older version of the project on their localhost. 🙄

@nelsonic
Copy link
Member Author

Note: Travis-CI is temporarily suspended, see: #55 (comment) related to: dwyl/learn-travis#67
But the tests all pass and the app works on localhost: #55 (comment)

Thanks for reviewing/merging. 👍

Copy link
Member

@ajmeese7 ajmeese7 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻 only exception is that I am getting the following error in my logs, and the image isn't loading in:

[debug] ** (Phoenix.Router.NoRouteError) no route found for GET /images/phoenix.png (ChatWeb.Router)

All the tests pass and the message functionality works though, so something may have just gone wrong in my environment. If you think the problem is localized I'm good to merge, for now I am going to approve of the PR.

Copy link
Member

@SimonLab SimonLab left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍
I don't have any log errors on my side.
@ajmeese7 maybe you can try to rebuild the app to see if it fixes the log on your side?

@nelsonic
Copy link
Member Author

@ajmeese7 you may need to run mix assets.deploy 💭

@nelsonic nelsonic merged commit 0731092 into main Oct 16, 2021
@nelsonic nelsonic deleted the Update-to-Phoenix-1.6-issue-#61 branch October 16, 2021 15:07
@ajmeese7
Copy link
Member

@SimonLab rebuilding the app worked, thank you for the suggestion!

Another great PR @nelsonic 🎉

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

Successfully merging this pull request may close these issues.

3 participants