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

Jason depp patch 20240801 #14

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Jason depp patch 20240801 #14

merged 2 commits into from
Aug 2, 2024

Conversation

JasonDepp
Copy link
Collaborator

Hi Limmek,

could you please review my updates and approve the pull request.

I know it's just tiny changes that I should report and let yourself update, consider this as a first time cooperation or a dry run.

if it works, then I can do more work in the future.

Thanks in advance.

rewrite code statement to make it more clear for future enhancement
- reorganize target verification and unit verification
- remove unit nil verification before setting local variables name and realm because it's already done ahead
- add comments
the CallbackHandler should come in before LibDataBroker and LibDBIcon because of dependencies, otherwise the addon can not load.
(test with this single addon living, turn off the rest)
@Limmek
Copy link
Owner

Limmek commented Aug 2, 2024

Looking good to me. Makes sense to merge the if statements for easier readability.

I refrained from altering the existing code while integrating the new Blizzard Menu into the retail version, hoping it would also be implemented in the classic versions, but it appears that will not happen.

I'm currently at my day job, so I'm unable to test it right now, but i will merge the changes.

@Limmek Limmek merged commit f663bd1 into Limmek:master Aug 2, 2024
1 check passed
@JasonDepp JasonDepp deleted the JasonDepp-patch-20240801 branch August 2, 2024 07:22
@JasonDepp
Copy link
Collaborator Author

Thanks.
I play only classic (in China it's 3.4.3 now), don't have coding knowledge and account to test it in retail.

do we have another channel to chat...
or just communicate via these tickets - -!

@Limmek
Copy link
Owner

Limmek commented Aug 2, 2024

You can contact on Discord.
Username: Limmek
ID: 336510254135377921
or join the community SWEDON u can find me there to.

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