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

Fix the return value of some os functions #1360

Merged
merged 3 commits into from
Nov 10, 2019

Conversation

sp-jordi-vilalta
Copy link
Contributor

@sp-jordi-vilalta sp-jordi-vilalta commented Oct 31, 2019

What does this PR do?

Fix the return values for os.rmdir and os.remove so they're consistent with the Premake documentation and the standard Lua ones.

How does this PR change Premake's behavior?

These functions now report some status that wasn't reported before, so the user can check it if needed. It was confusing not getting any result values in some cases, specially in a function that overrides the standard Lua library.

Anything else we should know?

I've just tested my changes on macOS. The test suite passes but I'm not sure if there may be anything that relies on the previous behavior (now there are some shortcuts when something can't be removed).
I also added a tiny mostly-unrelated commit to fix some comments on related files, I hope it's okay to have it here.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits

@samsinsane samsinsane merged commit fe962f8 into premake:master Nov 10, 2019
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.

3 participants