Skip to content

chore(future): rename Result interface to be consistent #122

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

Merged
merged 3 commits into from
Jul 13, 2025

Conversation

tech-voker
Copy link

@tech-voker tech-voker commented Jul 13, 2025

Aligns with existing naming pattern: Task/ResultTask, Pool/ResultPool, TaskGroup/ResultTaskGroup

Copy link

codecov bot commented Jul 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.57%. Comparing base (94b8d9a) to head (ef66172).
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #122   +/-   ##
=======================================
  Coverage   95.57%   95.57%           
=======================================
  Files          11       11           
  Lines         769      769           
=======================================
  Hits          735      735           
  Misses         31       31           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alitto
Copy link
Owner

alitto commented Jul 13, 2025

Hey @tech-voker,

Great catch 👍 . Since Result is being exported, renaming it would be a breaking change in packages using it explicitly. I think we can merge this as long as we keep the Result interface declared in future.go alongside the renamed ResultTask interface.
It would be nice if you could also add a specific test for this scenario as well. For instance, verifying that assigning the result of Submit() ResultTask[O] to a Result variable would work.
A deprecation comment needs to be added to Result as well for clarity.

@tech-voker
Copy link
Author

Hey @tech-voker,

Great catch 👍 . Since Result is being exported, renaming it would be a breaking change in packages using it explicitly. I think we can merge this as long as we keep the Result interface declared in future.go alongside the renamed ResultTask interface. It would be nice if you could also add a specific test for this scenario as well. For instance, verifying that assigning the result of Submit() ResultTask[O] to a Result variable would work. A deprecation comment needs to be added to Result as well for clarity.

Hi @alitto I have introduced the changes as suggested. Let me know if you need anything else to be covered in tests.

Copy link
Owner

@alitto alitto left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @tech-voker 🙌
I'll create a minor version release with these changes.

@alitto alitto merged commit 998332d into alitto:main Jul 13, 2025
15 checks passed
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.

2 participants