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

Rename internal File and OS resource classes #527

Merged
merged 3 commits into from
Mar 9, 2016
Merged

Conversation

arlimus
Copy link
Contributor

@arlimus arlimus commented Mar 9, 2016

This is a follow-up to #526 by @adamleff . Although it already prevents clashes on the global plane, I still think it's easy enough to accidentally call File inside a new resource and instead of getting ::File you get access to inspec's internal resource - which leads to potentially frustrating debugging. So as an additional layer to the MR ^^, we rename these resources and re-expose simple File calls, i.e. detect any problems early on.

@arlimus arlimus added the Type: Bug Feature not working as expected label Mar 9, 2016
This is just for simplicity. I expect other users to make the same mistake when using it, so I would rather our tests crash if we have this type of conflict again and prevent it in the first place. Renaming File to FileResource should take care of all important places
@chris-rock
Copy link
Contributor

This is a great simplification for developers. Thanks @arlimus

chris-rock added a commit that referenced this pull request Mar 9, 2016
rename File and OS resources
@chris-rock chris-rock merged commit 6819a46 into master Mar 9, 2016
@chris-rock chris-rock deleted the dr/file-and-os branch March 9, 2016 09:54
@arlimus arlimus changed the title rename File and OS resources rename inernal File and OS resource classes Mar 9, 2016
@arlimus arlimus changed the title rename inernal File and OS resource classes rename internal File and OS resource classes Mar 9, 2016
@arlimus arlimus changed the title rename internal File and OS resource classes Rename internal File and OS resource classes Mar 9, 2016
@adamleff
Copy link
Contributor

adamleff commented Mar 9, 2016

I had contemplated doing this but wasn't sure if it would be OK... and thought then maybe we should rename EVERYTHING to be consistent. Such a slippery slope. 😄

Thanks, @arlimus!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants