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.

Sign up for GitHub

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

Jump to bottom

make probot work with latest octokit deps #1864

Merged
merged 24 commits into from
Oct 29, 2023
Merged

make probot work with latest octokit deps #1864

merged 24 commits into from
Oct 29, 2023

Conversation

Uzlopak
Copy link
Collaborator

@Uzlopak Uzlopak commented Sep 29, 2023 •
edited by wolfy1339

note:

default webhookPath is now "/api/github/webhooks" and has to be overriden when migrating...

closes #1845, closes #1855, closes #1821

@Uzlopak Uzlopak requested a review from a team as a code owner September 29, 2023 01:17
@welcome
Copy link

welcome bot commented Sep 29, 2023

Thanks for opening this pull request! A contributor should be by to give feedback soon. In the meantime, please check out the contributing guidelines and explore other ways you can get involved.

@Uzlopak Uzlopak changed the title Refactor [WIP] Refactor Sep 29, 2023
@gr2m
Copy link
Contributor

gr2m commented Sep 29, 2023

There is some in progress to make tests pass again, see #1855

@gr2m gr2m mentioned this pull request Sep 29, 2023
Closed
@Uzlopak

This comment was marked as outdated.

Sign in to view
@gr2m
Copy link
Contributor

gr2m commented Sep 29, 2023 •
edited

Yes, sure. I'd prefer a beta branch as we expect breaking changes

Update: there you go https://github.com/probot/probot/tree/beta

@Uzlopak Uzlopak changed the base branch from master to beta September 29, 2023 20:55
@Uzlopak

This comment was marked as outdated.

Sign in to view
@Uzlopak

This comment was marked as outdated.

Sign in to view
@Uzlopak

This comment was marked as outdated.

Sign in to view
@Uzlopak

This comment was marked as outdated.

Sign in to view
@Uzlopak Uzlopak changed the title [WIP] Refactor update deps and fix the tests as much as possible Sep 29, 2023
Uzlopak
Uzlopak commented Sep 29, 2023
View reviewed changes
Copy link
Collaborator Author

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gr2m
@AaronDewes

I made my remarks. Please take a look :)

test/helpers/capture-log-output.ts

export async function captureLogOutput(action: () => any): Promise<string> {
export async function captureLogOutput(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems odd. Why couldnt we use the old implementation?

test/probot-octokit.test.ts Show resolved Hide resolved
src/server/server.ts Show resolved Hide resolved
src/octokit/get-probot-octokit-with-defaults.ts Show resolved Hide resolved
src/probot.ts Show resolved Hide resolved
src/apps/setup.ts Show resolved Hide resolved
src/apps/default.ts Outdated Show resolved Hide resolved
test/apps/default.test.ts Outdated Show resolved Hide resolved
test/server.test.ts Outdated Show resolved Hide resolved
test/e2e/e2e.test.ts Outdated Show resolved Hide resolved
@Uzlopak
Copy link
Collaborator Author

Uzlopak commented Oct 2, 2023

@gr2m

How can I support you for reviewing this PR?

gr2m
gr2m reviewed Oct 2, 2023
View reviewed changes
Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a first high-level review. Can you update the node version matrix in the test ci?

src/apps/setup.ts Show resolved Hide resolved
src/manifest-creation.ts Outdated Show resolved Hide resolved
src/octokit/get-authenticated-octokit.ts Outdated Show resolved Hide resolved
src/octokit/get-octokit-throttle-options.ts Outdated Show resolved Hide resolved
src/octokit/get-probot-octokit-with-defaults.ts Show resolved Hide resolved
@Uzlopak
Copy link
Collaborator Author

Uzlopak commented Oct 3, 2023

@gr2m

Addressed your remarks :)

@Uzlopak
Copy link
Collaborator Author

Uzlopak commented Oct 21, 2023

@gr2m
@wolfy1339

I think all your remarks are resolved. Just my three remarks at the beginning are left and needs feedback from you guys.

@gclhub
If you want to test this branch, than be aware, that you need probably set webhookPath accordingly to your needs ;)

@Uzlopak Uzlopak mentioned this pull request Oct 21, 2023
Closed
@wolfy1339 wolfy1339 mentioned this pull request Oct 21, 2023
Closed
@wolfy1339 wolfy1339 mentioned this pull request Oct 21, 2023
Closed
@wolfy1339 wolfy1339 mentioned this pull request Oct 22, 2023
Closed
@Uzlopak Uzlopak changed the title update deps and fix the tests as much as possible make probot work with latest octokit deps Oct 23, 2023
@Uzlopak
Copy link
Collaborator Author

Uzlopak commented Oct 23, 2023

@gr2m
@wolfy1339

I am quiete satisfied with this PR. I think it is ready for review/merge :)

@gclhub
Copy link

gclhub commented Oct 23, 2023

@gr2m @wolfy1339

I think all your remarks are resolved. Just my three remarks at the beginning are left and needs feedback from you guys.

@gclhub If you want to test this branch, than be aware, that you need probably set webhookPath accordingly to your needs ;)

I'll have some time to look at this today @Uzlopak

@Uzlopak
Copy link
Collaborator Author

Uzlopak commented Oct 23, 2023

@gclhub
Great! I am currently working on a MockAgent for nock fetch functionality. I hope that then i can finally upgrade and test our bots.

This was linked to issues Oct 29, 2023
Some payloads may not be parsed correctly #1784
Closed
Can't access payload of pull_request.opened #1848
Closed
This was unlinked from issues Oct 29, 2023
Some payloads may not be parsed correctly #1784
Closed
Can't access payload of pull_request.opened #1848
Closed
@wolfy1339 wolfy1339 linked an issue Oct 29, 2023 that may be closed by this pull request
Some payloads may not be parsed correctly #1784
Closed
@wolfy1339 wolfy1339 merged commit a6f4474 into probot:beta Oct 29, 2023
8 checks passed
@welcome
Copy link

welcome bot commented Oct 29, 2023

Thanks for your contribution to probot! 🎉
Congrats!

@github-actions
Copy link

🎉 This PR is included in version 13.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gr2m gr2m gr2m left review comments

@wolfy1339 wolfy1339 wolfy1339 approved these changes

Assignees
No one assigned
Labels
breaking change feature released on @beta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Some payloads may not be parsed correctly
4 participants
@Uzlopak @gr2m @gclhub @wolfy1339