Overhaul get-github-info - #2121
Conversation
🦋 Changeset detectedLatest commit: 175d608 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2121 +/- ##
==========================================
+ Coverage 85.34% 85.51% +0.17%
==========================================
Files 66 70 +4
Lines 2545 2548 +3
Branches 699 686 -13
==========================================
+ Hits 2172 2179 +7
+ Misses 343 338 -5
- Partials 30 31 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| commit: changeset.commit, | ||
| }); | ||
| if (info == null) { | ||
| // may be null if referenced the wrong repo, or the commit has not been pushed |
There was a problem hiding this comment.
thanks for the comment, this is useful stuff for future us ❤️
beeequeue
left a comment
There was a problem hiding this comment.
Add batch size as suggested by Chris on Discord. (This might indirectly fix the rate limit issue too?)
since batching the requests fixed it for them, i imagine their issue is that github only allows X queries in one GQL request
| .split("\n") | ||
| .map((l) => l.trimEnd()); | ||
|
|
||
| const links = await (async () => { |
There was a problem hiding this comment.
await (async () => {
truly cursed
| @@ -0,0 +1,127 @@ | |||
| import nock from "nock"; | |||
There was a problem hiding this comment.
i'd love if we could move from nock to mentoss, but when i tried last i ran into some issue i can't remember anymore...
fix #795
See the changesets for the main changes by this PR.
Context
Sorry for the huge PR. I ended up changing a lot of things here because:
nullfor TypeError: Cannot read properties of null (reading 'author') #795 case, I believe the function should return null/undefined and let users decide how to handle missing data. The leads to changing the function return type.index.tsbecome large and I split into multiple files.At some point I decided that many changes had already happen, so I just completely cleaned up the setup (in my opinion), which leads to also:
Potentially controversial
outdated note about returning markdown
Maybe we still want the APIs to return the Markdown directly? I agree that it'll make usage slightly less verbose, but I also think it's not too much work to construct in case they prefer formatting differently.
If we want to, maybe we can support
info.commit.formattedandinfo.pull.formattedor something.I have decided to return
markdownLinkfor the preformatted markdown.One thing left on the table is whether to still add compat APIs for
getInfoandgetInfoFromPullRequest(the old APIs). It shouldn't be hard to add if we want to.