feat: Front Matter support

[MattIPv4]

This introduces support for Front Matter (and threading) as raised in #166.

Implemented at present:

• Basic parsing for all Front Matter requested in the issue + validation
• Threading support (including Front Matter support for each thread item)
• Reply support
• (Quote) retweet support
• Media attachment support from local files
• Polls (needs Ads API access)

(This also adds support for invoking the library with --file flag, making development easier locally)

TODO:

• Body ^ syntax for attachments
• Media alt text support (covered by Use Twitter API v2 #195)
• Polls via v2 status API, not Ads (covered by Use Twitter API v2 #195)
• PR tests / Code coverage

Resolves #151
Resolves #134
Resolves #101
Resolves #92

[gr2m]

Thank you Matt! I hope to be able to look at your PR this Sunday! I'm excited to see others contribute, and to see the whole project move forward 🚀

[MattIPv4]

Awesome, thank you! I'll keep chipping away at the other functionality that's supported in the front matter (retweets, schedule, etc.) and push those up once done. I've been writing some tests for pushes based on the new stuff, but would love some feedback on getting the tests up to scratch (looks like PRs are also tested? and coverage is yelling at me).

[gr2m]

Sorry I run out of Sunday 😴 I am looking forward to review your work, as soon as I find some time, I won't forget. Please give me a few more days

[MattIPv4]

So, here's where we're at:

I've written all the logic for all the features that feel relatively simple to implement, and limited tests for all of them. (Except for the body syntax for attachments, I'll try to get that done soon:tm:.)

I'd love some more help on getting full test coverage for everything I've added (PR tests, plus improving the push tests as well).

Also worth noting that I've not actually tested any of this code against Twitter, only with the in-repo tests and referring to Twitter docs. It seems like testing this properly is either going to need faking a lot of GitHub data, or setting up a test repo using this branch in an action.

I'm not sure I really want to handle scheduling in this tweet. Looking at the Twitter docs for scheduling, it feels super limited unless I'm missing something -- you can schedule a tweet, but not anything else, so if a poll were to be added to a scheduled tweet, the poll would likely be expired when the tweet gets sent, as polls start from when they are created (at least, according to the docs).

My gut feeling on scheduling is that this'd be better handled through automation in GitHub, rather than relying on Twitter. E.g. automation that only merges a new tweet PR at the right time. That's why I'm feeling it'd be best not to tackle it as part of this PR, and move it to a separate discussion/issue/PR.

[gr2m]

I've written all the logic for all the features that feel relatively simple to implement, and limited tests for all of them. (Except for the body syntax for attachments, I'll try to get that done soon™️.)

Awesome!!

Also worth noting that I've not actually tested any of this code against Twitter, only with the in-repo tests and referring to Twitter docs. It seems like testing this properly is either going to need faking a lot of GitHub data, or setting up a test repo using this branch in an action.

We can use this repo and the twitter test account (https://twitter.com/commit2tweet/) for testing, no problem, I can share the credentials with you.

My gut feeling on scheduling is that this'd be better handled through automation in GitHub, rather than relying on Twitter. E.g. automation that only merges a new tweet PR at the right time. That's why I'm feeling it'd be best not to tackle it as part of this PR, and move it to a separate discussion/issue/PR.

Sounds like a good plan!

---

I'm still very underwater with life, and it will be this way until May. But things do clear up a little :) If you get blocked by anything please let me know and I'll try to find time to help out as soon as possible, otherwise I plan to have time for twitter-together in May. Wow, time flies!

[halkeye]

Where has this stalled out? Where can I help out at all? I'd really like to see some of these features for the jenkins project

[MattIPv4]

👀 Been a while since I've had brain time to allocate to this, but I believe my summary in the initial PR body is still correct. The big outstanding thing from what I can tell to be able to land this would be all the test writing that is needed to get coverage back to an acceptable point.

[gr2m]

sorry I did not forget about y'all, this PR is the oldest and now only remaining item on my TODO backlog. Please give me a few more days to catch up with new work from this week, I'm really looking forward to dive into this PR!

[IstoraMandiri]

We're using this fork over at https://github.com/ethereumclassic/twitter-together and appears to be working pretty well.

Haven't tested polls yet, and media tweets seems to be not working but will debug in the coming months.

[gr2m]

@MattIPv4 I'm very, very sorry for the delay.

As @IstoraMandiri is using it in production, I'm okay with merging the changes once we address the problem with media tweets

[MattIPv4]

👀 Taking a look at why media tweets don't work (waiting for an account to get approved for v1 API access to I can test on an actual account)

[MattIPv4]

Ah! Alt text is causing the issue as the twitter library this project uses doesn't support JSON payloads, which is required for that endpoint. I'll comment that out for now and it will likely be something that can be fixed when #195 is addressed.

[MattIPv4]

I should also note that polls should theoretically work for accounts with Ads API access, but I do not have an application that has access to that. Again, with #195 this can be solved for more properly by just using the direct polls option when creating a tweet via the v2 API.

[MattIPv4]

@gr2m I think this is probably good to go (I ran through all the new push to main tests using the file flag against my account, only polls fails as I don't have Ads API access), unless you want to look at adding more testing?

[gr2m]

@IstoraMandiri if you could give the latest version a few final tests, that'd be great, I'll try to do the same myself. Then I plan to merge on Monday if that's okay with both of you?

If you are interested in continuing to contribute, I'd be happy to invite you both as maintainers. No worries if not, just let me know.

[MattIPv4]

I'm not sure how much time I'd be able to throw at maintaining this on a regular basis, but am hoping to look at the v2 API update next, as it'll unblock a few things as a follow-up to this PR (and make the project more accessible w/o the need for applying for v1.1 or ads API access)

[IstoraMandiri]

Hey @gr2m sorry for the delay.

We did run into one parsing edge case in ethereumclassic/tweets-etc_network#39, seemingly to do with trailing spaces not being stripped.

We are probably going to be iterating with this in production in the coming weeks and would be happy to help maintain it, including implementing v2 and adding additional features with deletions being a must have.

Thanks @MattIPv4 for pointing out we can fix the media with the new API. I should have access to an account approved for testing polls in the next few days, hopefullly I'll be able to deep dive on this next week.

As we have an active community using this in production I'm sure we can help smooth out the rough edges and develop documentation etc.

https://github.com/ethereumclassic/twitter-together/issues

[MattIPv4]

Once this PR gets merged I'll follow up with a fix for that trailing whitespace 👍 Just needs some regex updated

[gr2m]

can you look into the failing tests? Otherwise this is good to go.

I think this might also be a good time to move the repository to the @twitter-together org. I'll invite you both as collaborators, not expectations though, just lowering barriers for further contributions.

[MattIPv4]

The only thing failing with tests is coverage, which I think would be a substantial chunk of further work to write out the PR test variants for every combination of FrontMatter.

Happy to do it here if you want that (will delay the PR for quite a while longer), or we can lower the coverage requirement for now and create a tech debt issue to restore full coverage?

[gr2m]

The only thing failing with tests is coverage

got it, let's lower the coverage requirement for now

[MattIPv4]

This should be good to go -- all existing tests in the repo are passing w/o modification, and all the new tests are passing too, so I'm hopeful this should be safe to land 💙

[gr2m]

Hey I just realized we didn't document all the new features yet. Can you update the README?

Are the remaining TODOs from the pull request body still up to date? We can create follow up issues once we merge

[MattIPv4]

🤦 Ah yes, will do some quick docs writing.

Yeah, those TODOs are up-to-date -- I'll file issues once this is merged (a few are already covered by existing issues)

[gr2m]

Thank you Matt for all the outstanding work, your patience with me, and welcome to the team :)

[gr2m]

I'm looking into the failing tests, the are related to the repository rename

[gr2m]

I think I got through all of them but one

npx tap test/push-main-setup/test.js

My kids just woke up, I won't be able to look into it before tomorrow. If you like and have the time, can you have a look? Maybe you figure out the last one?

[MattIPv4]

👀 Will have a look

[MattIPv4]

✅ Fixed -- so it looks like one of your recent commits to main as part of the rename bumped the lockfile to v2 badly (or the rebase here might've caused the bad state), when this branch is still expending lockfile v1 and Node.js 12.

We can properly bump the lockfile to v2 and start using Node.js 16 with #196

[MattIPv4]

(Interestingly the CI is already using Node.js 16 and is happy now, but if I switch to Node.js 16 locally and npm ci, the tests fail completely with an internal tap error)

[MattIPv4]

@gr2m If you're around enough to be able to merge this, that'd be ace ❤️ I'll jump right on doing the Node.js 16 upgrade properly to fix this weird lockfile situation

[gr2m]

@MattIPv4 shall we do a follow up issue for scheduled tweets explaining why we didn't add it and discuss there what might be done? I did create https://github.com/gr2m/merge-schedule-action for this very reason, but having it built-in would be nice. If a poll is the only limitation for scheduling, maybe we fail the CI if a scheduled tweet also includes a poll?

We can also update ethereumclassic/tweets-etc_network#41

[MattIPv4]

Let's take the discussion over to ethereumclassic/tweets-etc_network#41 and use that as the hub for the continued discussion of that 👍