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

Increased consistency of p. usage #757

Merged
merged 1 commit into from
Apr 25, 2017

Conversation

samsinsane
Copy link
Member

This is a pretty large change, but we went from ~900 usages of premake. to ~175. I probably went a bit overboard, and I'm happy to remove the ones where I added local p = premake.

@samsinsane samsinsane requested review from starkos and tvandijck April 25, 2017 06:49
Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

In theory, a harmless change. I thought we had removed the D module? LGTM, but I'll let @tvandijck have a quick look in case he has concerns with the size of the change.

local p = premake
local project = p.project
local config = p.config
local d = p.modules.d
Copy link
Contributor

Choose a reason for hiding this comment

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

tab vs spaces... may want to check other files as well...

Copy link
Member Author

Choose a reason for hiding this comment

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

The file was already using spaces, I feel like that change might be a bit unrelated to this PR. I'm happy to do up another PR if you'd like?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK... I think we should certainly make things consistent all around, according to the editorconfig file that is committed to the repository, and anyone committing to this repo should 100% use that tool in all their editors... so yeah, maybe a separate commit for that would be better.

@tvandijck
Copy link
Contributor

Change looks harmless to me too... merge it ;)

@tvandijck tvandijck merged commit 091d965 into premake:master Apr 25, 2017
@tvandijck
Copy link
Contributor

BTW... I'm pretty thankful you are willing to go through monkey work like this, so thank you very much.

@samsinsane samsinsane deleted the ssurtees/consistency branch April 25, 2017 16:38
@samsinsane
Copy link
Member Author

Haha, no worries! :)

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