-
Notifications
You must be signed in to change notification settings - Fork 961
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
feat: v13 #1874
Conversation
BREAKING CHANGE: Drop support for NodeJS < 18 BREAKING CHANGE: replace `node-fetch` with the Fetch API
This comment was marked as outdated.
This comment was marked as outdated.
fix linting of extensions.md
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Remove @tsconfig/node10 from devDepedencies as we are streamling with @octokit/tsconfig
chore: remove semver as dependency
* Set up code scanning This sets up CodeQL to automatically detect possible security problems. * Add more queries
…#1887) chore: simplify auth.ts
This updates all dependencies and replaces jest with vitest. Co-authored-by: wolfy1339 <[email protected]> Co-authored-by: Aras Abbasi <[email protected]> Co-authored-by: Gregor Martynus <39992 [email protected]> Co-authored-by: wolfy1339 <4595477 [email protected]>
What I'd like to do before merge (Am I missing anything here?):
After the release: |
Since Node 18, Headers is a global.
Huh, we should probably put this into Octokit 🤔 unless it has bad performance implications, but I'd only happen in the constructor so I don't think so? I don't mind leaving this in Probot to avoid the breaking change |
I dont have any clue, why the aliasLog was removed in the first place. Or why its type was called "DeprecatedLogger". I assume that there are some deeper thoughts behind that. Would be glad if you or @wolfy1339 could take care of it, because it is 1 a.m. in the morning and i have to grab some sleep. :D |
Fix typo in simulating-webhooks.md
Hey shall we ship v13? Anything that is left to do from your perspective? |
There is the log binding issue that should be addressed. Other than that, nothing seems to be blocking the release |
I could not reproduce the log binding error in a test. So it was not clear if the problem would be solved with that PR. |
I think I did that and removed it because it was called |
So should we merge #1939 just for the sake of it? |
I merged it. Maybe we can finally release :D |
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.
LGTM, let's go 🚀
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.
LGTM!
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.
@wolfy1339 all yours!
🎉 This PR is included in version 13.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This is necessary with probot@13, see probot/probot#1874 (comment).
This is necessary with probot@13, see probot/probot#1874 (comment).
This is a breaking change and hit my during my probot app update. General search was not helpful, so the only way to find it was to skim through all conversations to find that comment. |
I finally updated the WIP app to probot v13, thanks to UPDATE: had a quick look, throttling definitely remains enabled although I disable it e.g. here: Not sure why yet, might be a ProbotOctokit or even an Octokit bug with how defaults are set |
there is an open report here: #1972 |
BREAKING CHANGE: Drop support for NodeJS < 18
BREAKING CHANGE: replace
node-fetch
with the Fetch APIBREAKING CHANGE: default webhookPath is now
/api/github/webhooks
BREAKING CHANGE:
probot receive
now only supports payloads in JSON format, previously also (unintionally) allowed JS.closes #1643