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

Phx.1.7.10 fix #306

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Phx.1.7.10 fix #306

merged 1 commit into from
Apr 25, 2024

Conversation

drobban
Copy link
Contributor

@drobban drobban commented Jan 26, 2024

Related to issue #305

Passes available tests and seems to spin nice locally on my machine

Bumps kaffy version;
I think this might be the right move, as it might break backwards compat with older versions of Phoenix.

Changed deps versions to follow and fix the compile errors when using Kaffy with Phoenix 1.7.10 and phoenix_html 4.0

Would be awesome if more people could confirm & check this PR out.

drobban added a commit to drobban/kaffy that referenced this pull request Jan 26, 2024
As Phoenix 1.7.10 doesn't work just as is with Kaffy.

More info found in issue aesmail#305 
There is also a PR aesmail#306 that might fix 1.7.10 versions according to changelog stated https://hexdocs.pm/phoenix_html/changelog.html
@drobban drobban mentioned this pull request Jan 26, 2024
mix.exs Outdated Show resolved Hide resolved
@nathany-copia
Copy link

This update worked for me as well. Thanks @drobban.

@drobban
Copy link
Contributor Author

drobban commented Jan 26, 2024

This update worked for me as well. Thanks @drobban.

Thanks for testing it out!

@phortx
Copy link

phortx commented Mar 1, 2024

Tested. Seems to work for me too :)

@eriknaslund
Copy link

Seems very similar to #308, right?
We should probably just merge one of them.

@drobban
Copy link
Contributor Author

drobban commented Apr 10, 2024

Seems very similar to #308, right? We should probably just merge one of them.

look pretty identical to me =D

To me it doesn't matter which one gets merged if any.

I guess @aesmail has other priorities at the moment, the next best thing (until he has looked at the pr) is if people could test and run with the changes in this PR to make sure everything works as expected.

@mustela
Copy link

mustela commented Apr 21, 2024

I've been testing these changes for a couple of days already and no problems for now.

mix.exs Outdated Show resolved Hide resolved
@vitalis
Copy link

vitalis commented Apr 22, 2024

@aesmail @drobban
Can this please be released? Kaffy is a really great lib and thank you for it!!
Support of the latest libs will be an amazing feature :)

Changed deps versions to follow and fix the compile errors when
using Kaffy with Phoenix 1.7.10 and phoenix_html 4.0
@drobban
Copy link
Contributor Author

drobban commented Apr 23, 2024

@aesmail super!
Commit fixed to not bump kaffy version - still a bit rusty when it comes to git tooling, so just give a shout if I managed to screw something up, but it looks to be fine now (i think) 😅 .

@drobban drobban requested a review from aesmail April 23, 2024 13:15
@vitalis
Copy link

vitalis commented Apr 24, 2024

@aesmail Can you please review and release the update? Thank you

@drobban
Copy link
Contributor Author

drobban commented Apr 25, 2024

@aesmail Can you please review and release the update? Thank you

It will happen when he has time. While waiting, you are able to use this PR as a temporary dep in your project.

{:kaffy, github: "drobban/kaffy", ref: "66d0c7d"}
or simply fork my repo/branch and use that, then you wont risk the repo simply disappearing, but at the same time, if the branch disappears its a signal to switch back to upstream 😄

And when merged simply switch back to follow upstream again.

We cant forget that much of the opensource is peoples spare time spent making stuff that we are able to use for free. If we really need the service of getting things perfectly served as we would from a paid service... Well then "we" should consider to actually pay for it.

https://github.com/sponsors/aesmail
☮️ ❤️

@vitalis
Copy link

vitalis commented Apr 25, 2024

Dear @drobban , I will not do it for a few basic reasons. :)

First, we are human and often forget that we are using forks, especially when they work well. :)

Second, there's a thing called collaboration in the open source community; if someone has invested time to solve an issue or improve the product, there's no reason the project maintainer can't respond with gratitude and review/merge the PR as soon as possible, instead of waiting for a comfortable moment in life. :)

Additionally, this project hasn't been updated or maintained for a very long time. Usually, in the open source community, ownership is transferred to someone eager to advance the project.

I'd rather not use it at all than resort to using a fork. This also highlights that if something major requiring immediate attention occurs (as anything can happen), it will not be addressed.

And this is exactly why developers are leaving Elixir. The community...

Strong words, I know, but this is the reality.

@vitalis
Copy link

vitalis commented Apr 25, 2024

@aesmail Can you please review and release the update? Thank you

It will happen when he has time. While waiting, you are able to use this PR as a temporary dep in your project.

{:kaffy, github: "drobban/kaffy", ref: "66d0c7d"} or simply fork my repo/branch and use that, then you wont risk the repo simply disappearing, but at the same time, if the branch disappears its a signal to switch back to upstream 😄

And when merged simply switch back to follow upstream again.

We cant forget that much of the opensource is peoples spare time spent making stuff that we are able to use for free. If we really need the service of getting things perfectly served as we would from a paid service... Well then "we" should consider to actually pay for it.

https://github.com/sponsors/aesmail ☮️ ❤️

Yes, almost forgot.. I'm sure you and @aesmail sponsor every open source project that you use at work and at home.. ;)

@vitalis
Copy link

vitalis commented Apr 25, 2024

@aesmail Can you please review and release the update? Thank you

It will happen when he has time. While waiting, you are able to use this PR as a temporary dep in your project.
{:kaffy, github: "drobban/kaffy", ref: "66d0c7d"} or simply fork my repo/branch and use that, then you wont risk the repo simply disappearing, but at the same time, if the branch disappears its a signal to switch back to upstream 😄
And when merged simply switch back to follow upstream again.
We cant forget that much of the opensource is peoples spare time spent making stuff that we are able to use for free. If we really need the service of getting things perfectly served as we would from a paid service... Well then "we" should consider to actually pay for it.
https://github.com/sponsors/aesmail ☮️ ❤️

Yes, almost forgot.. I'm sure you and @aesmail sponsor every open source project that you use at work and at home.. ;)

I do "pay" for something that is a product and maintained: https://github.com/danschultzer

@drobban
Copy link
Contributor Author

drobban commented Apr 25, 2024

@aesmail Can you please review and release the update? Thank you

It will happen when he has time. While waiting, you are able to use this PR as a temporary dep in your project.
{:kaffy, github: "drobban/kaffy", ref: "66d0c7d"} or simply fork my repo/branch and use that, then you wont risk the repo simply disappearing, but at the same time, if the branch disappears its a signal to switch back to upstream 😄
And when merged simply switch back to follow upstream again.
We cant forget that much of the opensource is peoples spare time spent making stuff that we are able to use for free. If we really need the service of getting things perfectly served as we would from a paid service... Well then "we" should consider to actually pay for it.
https://github.com/sponsors/aesmail ☮️ ❤️

Yes, almost forgot.. I'm sure you and @aesmail sponsor every open source project that you use at work and at home.. ;)

dude... https://c.tenor.com/-Cr1JqBDtfIAAAAC/tenor.gif
Feel free to remind me if you find me trying to get VIP fast lane treatment.

Im just another dude making stuff for free as well, I have no more connection than you to this project.

@drobban
Copy link
Contributor Author

drobban commented Apr 25, 2024

Dear @drobban , I will not do it for a few basic reasons. :)

First, we are human and often forget that we are using forks, especially when they work well. :)

Second, there's a thing called collaboration in the open source community; if someone has invested time to solve an issue or improve the product, there's no reason the project maintainer can't respond with gratitude and review/merge the PR as soon as possible, instead of waiting for a comfortable moment in life. :)

Additionally, this project hasn't been updated or maintained for a very long time. Usually, in the open source community, ownership is transferred to someone eager to advance the project.

I'd rather not use it at all than resort to using a fork. This also highlights that if something major requiring immediate attention occurs (as anything can happen), it will not be addressed.

And this is exactly why developers are leaving Elixir. The community...

Strong words, I know, but this is the reality.

Having this conversation here is wildly off topic.
So this will be the last message of this character I'll respond to.

  1. I dont need you to be my white knight trying to force @aesmail to merge my PR. I was the one creating the patch - If I have issues with @aesmail, then its up to me if I want to have that discussion or not.
  2. Sure, its about collaboration. Have you tested this pull request out? Have you verified and gotten any results to report back? Sure I sincerely appreciate when I receive praises from people for work I do, but I do appreciate feedback and test results more.
  3. I just came home from Lisbon & the elixir euro conference. The community seems to be running strong, so I dont know what kind of reality you live in.

If you want to continue having this discussion, we can take it on IRC in #elixir @ libera.chat

Best regards
David
Peace out.

@vitalis
Copy link

vitalis commented Apr 25, 2024

@aesmail Can you please review and release the update? Thank you

It will happen when he has time. While waiting, you are able to use this PR as a temporary dep in your project.
{:kaffy, github: "drobban/kaffy", ref: "66d0c7d"} or simply fork my repo/branch and use that, then you wont risk the repo simply disappearing, but at the same time, if the branch disappears its a signal to switch back to upstream 😄
And when merged simply switch back to follow upstream again.
We cant forget that much of the opensource is peoples spare time spent making stuff that we are able to use for free. If we really need the service of getting things perfectly served as we would from a paid service... Well then "we" should consider to actually pay for it.
https://github.com/sponsors/aesmail ☮️ ❤️

Yes, almost forgot.. I'm sure you and @aesmail sponsor every open source project that you use at work and at home.. ;)

dude... https://c.tenor.com/-Cr1JqBDtfIAAAAC/tenor.gif Feel free to remind me if you find me trying to get VIP fast lane treatment.

Im just another dude making stuff for free as well, I have no more connection than you to this project.

So... if you not related to the project why do you write such comments? I politely asked for a simple thing.. and you just made everything so complicated.. I really don't get you "dude", I prefer m8.. more respectful ;)

@ghenry
Copy link
Collaborator

ghenry commented Apr 25, 2024

Hi all,

I also volunteer here as I use this in https://github.com/SentryPeer/SentryPeerHQ and an internal project at work. I helped review issues and PRs, so @aesmail gave me commit rights ages ago. If you use a project, see it needs help, this is one way you can help vs not sponsoring etc. You know how this works :-)

I've been busy too, so apologies for that.

Are we happy with this to merge then? There's no harm, we can revert it if not. Our users have tested and seem to be happy. Merging now.

Thanks all.

@ghenry ghenry merged commit d864ea1 into aesmail:master Apr 25, 2024
11 checks passed
@vitalis
Copy link

vitalis commented Apr 25, 2024

Hi all,

I also volunteer here as I use this in https://github.com/SentryPeer/SentryPeerHQ and an internal project at work. I helped review issues and PRs, so @aesmail gave me commit rights ages ago. If you use a project, see it needs help, this is one way you can help vs not sponsoring etc. You know how this works :-)

I've been busy too, so apologies for that.

Are we happy with this to merge then? There's no harm, we can revert it if not. Our users have tested and seem to be happy. Merging now.

Thanks all.

Thank you!!!!!

@vitalis
Copy link

vitalis commented Apr 25, 2024

Thank you @drobban !!, I really appreciate your investment in this project

@ghenry
Copy link
Collaborator

ghenry commented Apr 25, 2024

Remember, this isn't a release yet.

@drobban drobban deleted the phx_1.7.10_breaking branch April 25, 2024 10:19
@Jaza
Copy link

Jaza commented Jul 28, 2024

Thanks @drobban for the fix. @aesmail @ghenry any chance we can get a new release? Latest is still kaffy 0.10.2, which gives me an error when I try to install it (with phoenix 1.7.14).

@ghenry
Copy link
Collaborator

ghenry commented Jul 28, 2024

Will take a look tonight. Thanks for the PR tag!

@TheArrowsmith
Copy link

I hate to pester, especially in light of what's already been written above, but are there any updates on getting this PR into a release?

I appreciate that you guys are volunteers doing this for free. But the latest version of Phoenix depends on phoenix_html ~> 4.1, which means I can't install Kaffy on a newly-generated Phoenix app:

$ mix deps.get
Resolving Hex dependencies...
Resolution completed in 0.07s
Because your app depends on kaffy 0.10.2 which depends on phoenix_html ~> 3.0, phoenix_html ~> 3.0 is required.
So, because your app depends on phoenix_html ~> 4.1, version solving failed.
** (Mix) Hex dependency resolution failed

I realise that I can update my deps to pull the working version of Kaffy directly from Github, but most people trying Kaffy aren't going to get that far. Not having this PR in a release will really hinder Kaffy's adoption.

@drobban
Copy link
Contributor Author

drobban commented Oct 4, 2024

I hate to pester, especially in light of what's already been written above, but are there any updates on getting this PR into a release?

I appreciate that you guys are volunteers doing this for free. But the latest version of Phoenix depends on phoenix_html ~> 4.1, which means I can't install Kaffy on a newly-generated Phoenix app:

$ mix deps.get
Resolving Hex dependencies...
Resolution completed in 0.07s
Because your app depends on kaffy 0.10.2 which depends on phoenix_html ~> 3.0, phoenix_html ~> 3.0 is required.
So, because your app depends on phoenix_html ~> 4.1, version solving failed.
** (Mix) Hex dependency resolution failed

I realise that I can update my deps to pull the working version of Kaffy directly from Github, but most people trying Kaffy aren't going to get that far. Not having this PR in a release will really hinder Kaffy's adoption.

Im not sure how the release "cycle" looks like. It does seem like the pr is merged into master. But it is as you say - the most probable experience is that a new users just selects the latest release and try to use it with the latest release of phoenix and that will make people believe that this project is broken.

At the same time - I have no constructive suggestion on how to make this project follow the release cycle of phoenix - This isnt the first time something breaks as Phoenix changes their structure, it will probably not be the last.

This is my second PR to keep it up date with Phoenix and both times was caused by "structural" changes.

@ghenry
Copy link
Collaborator

ghenry commented Oct 4, 2024 via email

@bryszard
Copy link

bryszard commented Oct 13, 2024

@ghenry I'm the new user here, trying to install the kaffy on the newest Phoenix, and I'm having the experience as described by @drobban and @TheArrowsmith.

I'd appreciate if I could use the official version of kaffy with a fix instead of a commit.

Thanks in advance!

@vitalis
Copy link

vitalis commented Oct 13, 2024

@ghenry, @drobban, @TheArrowsmith just install the master branch version

@drobban
Copy link
Contributor Author

drobban commented Oct 16, 2024

@ghenry I'm the new user here, trying to install the kaffy on the newest Phoenix, and I'm having the experience as described by @drobban and @TheArrowsmith.

I'd appreciate if I could use the official version of kaffy with a fix instead of a commit.

Thanks in advance!

You can use the official version of kaffy, but you have to point at the github repo and preferably pin it to a commit instead of using the package.

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.