Skip to content

Switch to RailsGravatar#881

Merged
glebm merged 1 commit intothredded:masterfrom
mission-met:swap-gravatar
Apr 5, 2021
Merged

Switch to RailsGravatar#881
glebm merged 1 commit intothredded:masterfrom
mission-met:swap-gravatar

Conversation

@rickychilcott
Copy link
Copy Markdown
Contributor

This addresses #880 which is a concern about relying on the rb-gravatar gem, which has been GPLed in sn/ruby-gravatar@5a1225c We could peg our dependency on an older version of the gem, but it might be better to rely on a gem that is guaranteed to not be GPLed.

As such, I created a gem called RailsGravatar which has a very similar API but changes some of the internals to better output rails tags (using ActionView helpers) and wrote a comprehensive test suite. The most important part is that it's MIT licensed and has no concerns about it being GPLed.

@glebm glebm merged commit 1d3525f into thredded:master Apr 5, 2021
@glebm
Copy link
Copy Markdown
Collaborator

glebm commented Apr 5, 2021

Thanks!

@glebm
Copy link
Copy Markdown
Collaborator

glebm commented Apr 10, 2021

Looks like this isn't working correctly (CI failing)

The 3rd argument to rb-gravatar's src was 'mm' but RailsGravatar expects a URL?

Can you please send a PR fixing this?

@glebm
Copy link
Copy Markdown
Collaborator

glebm commented Apr 10, 2021

Looking at the source, it's not the fallback_image_url; not sure what the issue is

@glebm
Copy link
Copy Markdown
Collaborator

glebm commented Apr 10, 2021

Example failing job https://travis-ci.org/github/thredded/thredded/jobs/766574511

Error:

   Failure/Error: <%= image_tag post.avatar_url, class: 'thredded--post--avatar' if post.user %>

      ActionView::Template::Error:

        nil is not a valid asset source

@timdiggins
Copy link
Copy Markdown
Collaborator

@rickychilcott we can't reopen this PR because it was merged once, but can you open another PR for this when you're ready?

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.

3 participants