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

fix(twitter-card): use proper image tag #7025

Merged
merged 1 commit into from Jun 30, 2016

Conversation

gergelyke
Copy link
Contributor

@gergelyke gergelyke commented Jun 22, 2016

The reason:

https://dev.twitter.com/cards/types/summary

  • Commit message has a short title & issue references
  • Commits are squashed
  • The build will pass (run npm test).

@acburdine
Copy link
Member

Hey @gergelyke!

I can't say whether or not this can be merged, but you also need to re-write some of the tests. If you look at the travis failures it should show you which ones you need to edit.

Thanks! 😄

@gergelyke
Copy link
Contributor Author

Hello @acburdine ,

got it, fixing it now!

G

@gergelyke
Copy link
Contributor Author

It is fixed - how can we make sure it is merged? Currently, the Twitter share card is not working with Ghost.

@acburdine
Copy link
Member

acburdine commented Jun 23, 2016

Thanks! I'll take a look in a bit and verify it.

What I meant by that earlier is that I wasn't sure if I myself should merge it. If it's not working with twitter currently though, I'll definitely check it. (I normally don't merge backend changes, I'm more of a frontend person)

@gergelyke
Copy link
Contributor Author

It works with facebook properly, but not with twitter. It should be merged asap.

@gergelyke
Copy link
Contributor Author

any chance we can merge it today?

@acburdine
Copy link
Member

@gergelyke Even if we did, you'd still have to wait until the next release of ghost to use it in production 😄

/cc @ErisDS

@gergelyke
Copy link
Contributor Author

When would that happen?

@acburdine
Copy link
Member

Hopefully soon, although the next release is a pretty big one so it might be another week or so.

@gergelyke
Copy link
Contributor Author

Got it ,thanks!

@acburdine
Copy link
Member

@gergelyke Sorry for taking so long (had to figure out how to test it correctly).

This looks good 👍

@acburdine acburdine merged commit 447cc0c into TryGhost:master Jun 30, 2016
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
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.

None yet

2 participants