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 phoenix 1.4 #400

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

norbajunior
Copy link

@norbajunior norbajunior commented Nov 22, 2018

Upgrade Phoenix to ~> 1.4

  • Upgraded elixir to 1.7 since it's is a requirement for phoenix 1.4

  • Replaced Poison lib in favor of Jason one

  • A Routes alias has been added to coherence_web.ex for view and controller blocks in favor over the previously imported AppWeb.Router.Helpers. In reason of that
    all routes around the lib was updated to Routes.path_name_path

Upgrade ecto to ~> 3.0

  • Ecto.Changeset.cast/3 deprecated usage of binary keys,
    so a refactoring was applied to solve this.
    warning: non-atom keys in cast/3 is deprecated. All keys must be atoms, got:"reset_password_token"

  • Removed timex_ecto lib since ecto 3.0 does not support it

  • Passing :adapter via config is deprecated in favor of passing it on use Ecto.Repo

  • Use Ecto.Migrator.migrations_path/1 in favor of Ecto.migrations_path/1 that was removed

  • postgrex lib ~> 0.14.0

@norbajunior norbajunior force-pushed the upgrade-to-phoenix-1.4 branch 4 times, most recently from aa3b0b6 to 23367b0 Compare November 22, 2018 12:29
* Ecto.Changeset.cast/3 deprecated usage of binary keys,
  so a refactoring was applied to solve this.
`
warning: non-atom keys in cast/3 is deprecated. All keys must be atoms, got: `"reset_password_token"`
`

* Removed timex_ecto lib since ecto 3.0 does not support it

* Passing :adapter via config is deprecated in favor of passing it on use Ecto.Repo

* Ecto.migrations_path/1 was moved to Ecto.Migrator module

* postgrex lib ~> 0.14.0
* Upgraded elixir to 1.7 since it's is a requirement for phoenix 1.4

* Replaced Poison lib in favor of Jason one

* A Routes alias has been added to coherence_web.ex for view and controller blocks in favor over the previously imported AppWeb.Router.Helpers. In reason of that
all routes around the lib was updated to Routes.path_name_path
@ashishbista
Copy link

ashishbista commented Dec 4, 2018

Any chances to get it merged?

@JesperWe
Copy link

JesperWe commented Dec 4, 2018

It's slightly puzzling why there is no action here? 1.4 has been out almost a month now... Maintainer went on honeymoon ;-) ? Maybe some other contributor could pick it up? @manuel-rubio @klacointe

@maratgaliev
Copy link

Hey, any chances to merge this? Guys please take a look.

@Sjoerd
Copy link

Sjoerd commented Dec 10, 2018

Will this be merged?

@manuel-rubio
Copy link
Contributor

I guess... but keep in mind not all of us changed all of our projects to Phoenix 1.4 and Ecto 3.0. I think it should be released as the master branch with a change in the high number of version and keep a branch for the previous version to let us maintain for a while the old branch just in case some issues appear.

@JesperWe JesperWe mentioned this pull request Dec 24, 2018
@MattGaud2425
Copy link

This would be great

@Sjoerd
Copy link

Sjoerd commented Jan 7, 2019

@smpallen99 please merge this.

@kevinkjt2000
Copy link

kevinkjt2000 commented Jan 9, 2019

I found that some of the generated template files do not have Routes. in front of the _path functions. Phoenix 1.4 seems to want this to be the case, so should that be updated in the places that generate these files before this is merged?

I ended up fixing each of these files one-by-one after running mix compile:

        modified:   lib/sample_web/templates/coherence/invitation/edit.html.eex
        modified:   lib/sample_web/templates/coherence/invitation/new.html.eex
        modified:   lib/sample_web/templates/coherence/password/edit.html.eex
        modified:   lib/sample_web/templates/coherence/password/new.html.eex
        modified:   lib/sample_web/templates/coherence/registration/edit.html.eex
        modified:   lib/sample_web/templates/coherence/registration/new.html.eex
        modified:   lib/sample_web/templates/coherence/registration/show.html.eex
        modified:   lib/sample_web/templates/coherence/session/new.html.eex
        modified:   lib/sample_web/templates/coherence/unlock/new.html.eex

@norbajunior
Copy link
Author

@kevinkjt2000 These files should be fixed. I'll fix them. Thanks to spot that.

@norbajunior
Copy link
Author

norbajunior commented Jan 10, 2019

Hi guys,

The routes in templates are fixed now. I also created a demo project https://github.com/norbajunior/coherence_demo using the branch of this PR and it's working fine.

@smpallen99 feel free to check out this demo to help you to validate the PR.

Cheers.

@JesperWe
Copy link

I am wondering if smpallen99 is still around...? There have been no commits or comments for 3 months???

@norbajunior
Copy link
Author

norbajunior commented Jan 10, 2019

@JesperWe I just sent an email to @smpallen99 about the PR. I hope the email reaches him.

@OscarBarrett
Copy link

@norbajunior Looks like this one was missed

@norbajunior
Copy link
Author

@OscarBarrett thanks for the help. Done in this commit 10bb848

@natecox
Copy link

natecox commented Jan 14, 2019

FWIW, this PR is mandatory for my adoption of coherence on a new project. I did a {:coherence, github: "appprova/coherence", ref: "10bb848f885217097ac5be76b202d9bf213b7e20"} to get up and running which works fine, but I would love to see this released as a major version ASAP.

@erniedeferia
Copy link

Anyone heard if maintainer(s) plans on merging this PR?

@narrowtux
Copy link

Is it necessary to use ecto_sql as a dependency? I can't stay on ecto 2.x temporarily while updating phoenix to 1.4 now.

@Sjoerd
Copy link

Sjoerd commented Mar 26, 2019

@narrowtux I don't think that we will ever have an update... @smpallen99 is inactive for over 3 months now..

@norbajunior
Copy link
Author

norbajunior commented Mar 29, 2019

As it seems @smpallen99 is inactive I merged this PR into my repo master branch.

So you guys can install it getting from https://github.com/norbajunior/coherence for now:

def deps do
  [{:coherence, github: "appprova/coherence"}]
end

@popo63301
Copy link

Hi guys ! If you are interested about this project and want to take part of maintaining it, @smpallen99 is looking for people to create a core team: https://elixirforum.com/t/looking-to-create-exadmin-coherence-core-teams/10641 . It would be great to have people who continue improving this library.

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

Successfully merging this pull request may close these issues.