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

[WIP] Audit return types #724

Closed
wants to merge 5 commits into from
Closed

[WIP] Audit return types #724

wants to merge 5 commits into from

Conversation

aaronayres35
Copy link
Contributor

@aaronayres35 aaronayres35 commented May 3, 2021

In this PR I try to make a bunch of changes at once to input / return types across chaco and see what happens.

Note the changes in this PR could fix (potentially) #272, #289, #550, and more. Eventually I will verify these things and try to write regression tests.

I currently am treating this PR as an experiment. I imagine the changes here will likely be pulled out into various, more incremental, small PRs that slowly make a migration. If this PR ever is to be merged in its entirety, it will require substantial clean up and tests.

A struggle with making these changes is lack of coverage (we could be breaking other things internally even if test suite passes), and Hyrum's law. Almost certainly downstream users are depending on these return types in one way or the other. The issue description on #550 very nicely describes how these differences across the codebase force other code to assume one way or the other... Users likely have had to make that assumption that holds true for their particular use case.

@rahulporuri rahulporuri self-requested a review May 5, 2021 12:28
@aaronayres35
Copy link
Contributor Author

I am going to close this now as I do not intend for it to ever be merged, and #726 exists as a first step

@rahulporuri rahulporuri deleted the audit-return-types branch June 16, 2021 12:28
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.

1 participant