-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: Switch to the v2 Twitter API #207
Conversation
lib/common/tweet.js
Outdated
// } | ||
|
||
return id; | ||
const mediaId = await client.v1.uploadMedia(file, { mimeType: mime.lookup(file) }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the v2 API does not support media uploads?
Will twitter accounts without v1 access be unable to upload media?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I note in the description above:
This still uses the v1 API for media, but essentials API access will allow access to this as there is no v2 equivalent currently
Only remaining work here now is to update the README and docs directory to remove references to the Ads API, to remove guidance on requesting elevated access as only essentials access is required now (v2 + v1 media), and to remove the comment that alt text cannot be set as it now can be. |
I have tested each tweet type's pull request flow in production over at https://github.com/IstoraMandiri/twitter-together-testing. The twitter account only has v2 API access, but required setting up read/write permissions in a way that is different to the current docs. This should be updated. I was able to confirm that this branch seems to be working as expected other than some PR preview bugs and misc improvements, for which I have added issues in this repo. |
5cd0ee0
to
184845c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work!!
One more thing we need to change is the release.yml workflow here:
action/.github/workflows/release.yml
Line 23 in dffa59c
HEAD:refs/heads/v2 |
This won't be necessary once I get to work on #205 😢
Should I bump package.json to 3.0.0 at the same time, or should I leave that for the bot to handle still? |
that should be done by semantic-release via this configuration: Lines 60 to 67 in dffa59c
|
Cool, will bump the workflow, pls hold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀:shipit:
Nice, can confirm the Twitter App creation docs are also looking very good and match my recent experience. |
@gr2m Is there anything blocking merging this? Wasn't sure if I can boop the button, or if you'd left it unmerged for a reason? |
Usually I leave it up to the author to merge the pull request at their convenience, especially for more complex pull requests. So yeah, when things are green, boop away at your pleasure |
🚀 booped |
🎉 This PR is included in version 2.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
:sus: Why did it release it as minor |
Gregor, can you do some magic to unrelease that minor release, 'cause this is very likely a breaking change for some folks 🤔 |
Most users should be safe, it's only breaking for those that're using apps outside a project, as legacy apps only have v1 access. (I think, I also could be totally wrong there) I'm guessing the bot didn't look at the PR body, only the commit body (which just had the normal commit log w/o the breaking change warnings). Automation 😓 |
I notice that the dist build is not lined up with the release's semver.
Perhaps bump the package.json to 3.0.0 and rebuild? |
Ouch, sorry :( semantic-release needs a As we don't release to npm, we can just delete the git tag/github release and have semantic-release do a breaking release with an empty commit. I'm on it |
🎉 This PR is included in version 2.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
the above comment makes no sense to me 🤷🏼 Here are the two relevant releases
2.x maintenance releases can be done with pull requsets against the |
Ty for the fix -- yah, figured it was probably the lack of breaking change mention in the commit. I'd read the doc before but I think parsed it as needing breaking change in the PR body, not commit body (though commit makes far, far more sense). |
semantic-release only works on |
Makes total sense, I just had a lapse in brain function 😂 |
To Istora's point above though, https://github.com/twitter-together/action/blob/v3/dist/index.js does declare itself as v2 still though 'cause it was built before the package.json bump was done. Is there a semantic-release plugin that can run the build command after the version bump, and then commit both the updated package.json and dist.js? (Which I think is really all that's needed to solve for #205) |
I think this can be done via https://github.com/semantic-release/git |
breaking change: Moves to using the v2 API. Twitter apps need to be in a project to access the v2 API.
breaking change: When retweeting, the only URL returned now is the original tweet, not the retweeter's tweet.
Replaces the
twitter
library withtwitter-api-v2
, and switches the logic over to using the v2 Twitter API where possible.This removes the need for the Ads API as the v2 tweet endpoint supports polls directly. This still uses the v1 API for media, but essentials API access will allow access to this as there is no v2 equivalent currently.