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.

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

Revert "Reduce usage of "any" (#448)" #469

Merged
merged 1 commit into from
May 23, 2024
Merged

Revert "Reduce usage of "any" (#448)" #469

merged 1 commit into from
May 23, 2024

Conversation

kraftp
Copy link
Contributor

@kraftp kraftp commented May 23, 2024

This reverts commit 8f1b5d4 as it included breaking API changes. It caused numerous failures on our demo applications and in our staging environment and cannot be released in its current state. The changes in that PR are good, but we need to introduce them gradually and carefully (likely in multiple smaller PRs) and verify they don't cause regressions.

@demetris-manikas
Copy link
Contributor

I just submitted a fix for that

@chuck-dbos chuck-dbos self-requested a review May 23, 2024 22:13
@kraftp
Copy link
Contributor Author

kraftp commented May 23, 2024

I just submitted a fix for that

Unfortunately there were other issues, such as #463. More broadly, it's possible to write code like:

await ctx.setEvent<PresignedPost>("uploadkey", mkey);

Some apps have code like this (such as https://github.com/dbos-inc/dbos-demo-apps/tree/main/yky-social), and #448 broke them because those functions aren't generic any more. We shouldn't change public interface signatures in a non-backwards-compatible way. However, like I said, many of the changes in #448 were good (especially the internal changes) and we'd be happy to work with you to re-introduce them in smaller PRs. Thanks a lot for your contributions, we really appreciate them!

@kraftp kraftp merged commit f8cc4c6 into main May 23, 2024
2 checks passed
@kraftp kraftp deleted the kraftp/revert-448 branch May 23, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants