[go: nahoru, domu]

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

Simplify checking depend #1

Merged
merged 2 commits into from Nov 5, 2016
Merged

Simplify checking depend #1

merged 2 commits into from Nov 5, 2016

Conversation

ghost
Copy link
@ghost ghost commented Nov 5, 2016

And some tweaks...

By the way, I want to transfer carpets to you, my version will be no longer maintained.
Is it OK with you?

@bell07
Copy link
Collaborator
bell07 commented Nov 5, 2016

Thank you for your contribution. It is ok for me if you transfer carpets to me and I maintain them in the feature. I do not thing there will be bug- or feautre- requests as so much.
I see you removed the modutils.lua. I use them in a second mod too so it is a reusable functionality for me. Do you see a way to optimize the code in modutils.lua in a reusable way? Or do you see the reusable approach is oversized for this ~20 (~60) lines?

@ghost
Copy link
Author
ghost commented Nov 5, 2016

I think they shouldn't be in global.
https://github.com/Rui-Minetest/minetest-carpets/compare/patch-1...Rui-Minetest:p
How about this way?

@bell07
Copy link
Collaborator
bell07 commented Nov 5, 2016

Yes! I like this way! Now I need to update the other mods too to use the modutils in this way.
The functionality is lost if the dependency is required or optional (depentry.required in my implementation). But I don't need this currently in other mods too.

@bell07 bell07 merged commit 4f16b49 into minetest-mods:master Nov 5, 2016
@ghost
Copy link
Author
ghost commented Nov 5, 2016

These changes weren't in this PR yet...
I'll new PR for it.

@bell07
Copy link
Collaborator
bell07 commented Nov 5, 2016

I merged it already without PR ;)

@ghost
Copy link
Author
ghost commented Nov 5, 2016

Thanks.

@ghost
Copy link
Author
ghost commented Nov 5, 2016

I transfered my version to you. It’s all yours. If you don't need it, remove it free.

@ghost
Copy link
Author
ghost commented Nov 5, 2016

Argument of check_depend is modname, please be careful.

@bell07
Copy link
Collaborator
bell07 commented Nov 5, 2016

Ok. I renamed it to bell07/minetest-carpets-legacy
Can you please update your thread in "Mod Releases" in minetest forum?

@ghost
Copy link
Author
ghost commented Nov 5, 2016

I updated.
The thread will be moved to "Old Mods" section.

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.

None yet

1 participant