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

Ensure Dapr workflows classes and interfaces have proper packages and visibility #1176

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

artur-ciocanu
Copy link
Contributor

Description

Ensure Dapr workflows classes and interfaces have proper packages and visibility

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1175

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@artur-ciocanu artur-ciocanu requested review from a team as code owners December 8, 2024 08:58
@artur-ciocanu artur-ciocanu marked this pull request as draft December 8, 2024 09:13
@artur-ciocanu artur-ciocanu marked this pull request as ready for review December 8, 2024 10:53
@artur-ciocanu
Copy link
Contributor Author

@artursouza @salaboy and @cicoyle could you please take a look. Although there are a lot of files, all the changes are just moving classes around and renaming them. These are breaking changes for the users of Dapr Workflows, but I think it aligns better with the intended usage and programming model. Also since we are still in BETA for workflows it should be OK.

Please check it out and let me know your thoughts.

Thank you!

@salaboy
Copy link
Contributor

salaboy commented Dec 9, 2024

/lgtm ! please review and merge

@artur-ciocanu
Copy link
Contributor Author

@artursouza and @cicoyle I have addressed the comments. Could you please take another look?

Thank you!

@salaboy
Copy link
Contributor

salaboy commented Dec 10, 2024

/lgtm!

@artur-ciocanu
Copy link
Contributor Author

@artursouza and @cicoyle whenever you have a chance could you please review and approve. Thank you!

@artur-ciocanu
Copy link
Contributor Author

@artursouza and @cicoyle sorry to ping you again, could you please review and appprove.

@artur-ciocanu
Copy link
Contributor Author

@artursouza and @cicoyle could you please review and approve. Thank you!

@salaboy
Copy link
Contributor

salaboy commented Dec 19, 2024

/approve

Copy link
Member

@artursouza artursouza left a comment

Choose a reason for hiding this comment

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

Just revert the docs changes and LGTM.

@artur-ciocanu
Copy link
Contributor Author

@artursouza I have addressed the comments. Could you please approve. Thank you!

@artur-ciocanu
Copy link
Contributor Author

@artursouza and @cicoyle whenever you have a chance could you please approve. Thank you!

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.

Ensure Dapr workflows classes and interfaces have proper packages and visibility
4 participants