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

Adding NyzulHelper addon. #1930

Merged
merged 4 commits into from
Sep 11, 2020
Merged

Adding NyzulHelper addon. #1930

merged 4 commits into from
Sep 11, 2020

Conversation

GlarinAsura
Copy link
Contributor

Initial commit of NyzulHelper addon.

Initial commit of NyzulHelper addon.
Copy link
Member

@z16 z16 left a comment

Choose a reason for hiding this comment

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

For the most part the addon looks fine, function-wise. Stylistically, since we try to go for a uniform style please use 4 spaces for one level of indentation, use single quotes for strings and add a new line at the end of the file, and prefer multi-line if over single-line.

Also, for the addon to be listed in the launcher you need to add an entry into the addons.xml file. You can copy the structure for other addons in there.

I wish I could provide some insight into the functional aspect as well, but I know too little about Nyzul to contribute, so I'll trust your judgment here.

addons/NyzulHelper/NyzulHelper.lua Outdated Show resolved Hide resolved
addons/NyzulHelper/NyzulHelper.lua Outdated Show resolved Hide resolved
addons/NyzulHelper/NyzulHelper.lua Outdated Show resolved Hide resolved
Fixed incorrect text in disclaimer.
Made indentation uniform.
Switched all string literals to use single qoutes.
Made suggested help command change.
@GlarinAsura
Copy link
Contributor Author

Thank you so much for taking the time to review my code. I have made all of the requested changes.

@GlarinAsura GlarinAsura requested a review from z16 September 10, 2020 04:15
@z16 z16 merged commit 04e966b into Windower:dev Sep 11, 2020
z16 added a commit that referenced this pull request Nov 4, 2024
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