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

Refactor Side Load of ZendeskAPI::Association Into A Separate Object #555

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

Conversation

khezam
Copy link

@khezam khezam commented Aug 21, 2023

It seems like the side load behaviors of the ZendeskAPI::Association class could be encapsulated in a separate object for extendability so I thought it might be a good opportunity to refactor.

Let me know if it's not needed and I'm happy to close this PR.

@khezam khezam changed the title Refactor Side Load of Association Class Into A Separate Object Refactor Side Load of ZendeskAPI::Association Into A Separate Object Aug 21, 2023
@khezam khezam marked this pull request as ready for review August 21, 2023 22:10
@khezam khezam requested a review from a team as a code owner August 21, 2023 22:10
@ecoologic
Copy link
Contributor

Hi @khezam - thank you for another contribution! (your previous one is coming, I just need another internal review).

We don't have much time at present to do a deep review on it, but eventually we'll find the time.

First question: Is this code already covered by existing tests or would it benefit from extra testing?

Thanks again ❤️

@khezam
Copy link
Author

khezam commented Aug 21, 2023

Hi @ecoologic - my pleasure and I'm happy I could help :)

We don't have much time at present to do a deep review on it, but eventually we'll find the time.

No worries at all and I totally understand that.

First question: Is this code already covered by existing tests or would it benefit from extra testing?

Yes, this code is already covered by existing tests :)

@khezam khezam force-pushed the khezam/refactor_side_load branch from 498742a to 6fd2ba6 Compare August 23, 2023 12:57
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