Skip to content

Standardise file naming, header guards #99

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

Closed
wants to merge 5 commits into from

Conversation

willgittoes-dd
Copy link
Contributor

@willgittoes-dd willgittoes-dd commented Sep 4, 2018

Standardises source file naming on TitleCase.ext, which is more windowsy than the alternative snake_case.ext.

Same deal with header guards. There was a mix of #defines and #pragma onces, and I settled on the latter as being more common in windows code.

@willgittoes-dd willgittoes-dd added the area:builds project files, build scripts, pipelines, versioning, releases, packages label Sep 4, 2018
@lucaspimentel
Copy link
Member

@willgittoes-dd: You and @dd-caleb should agree on one style for filenames and #pragma once. Looks like you both are moving things in different directions. Caleb has been renaming file from CamelCase to snake_case, recently, and changing #pragma once to #ifdef. Let's just pick one style guide (Google C++ Style Guide?) and stick to it.

And don't worry about the C++ code being more "Windows-y." It will need to compile on Linux at some point, too.

@dd-caleb
Copy link
Contributor

dd-caleb commented Sep 4, 2018

Yeah I had been removing the pragmas and renaming the files to match the google style guide. Don't have a strong opinion one way or the other, but if we use pragma once we should make sure it works in non-windows.

@willgittoes-dd
Copy link
Contributor Author

ehehe, I'm happy to flip this to doing it all the other way :) I picked these conventions because they were the ones used in the ClrSamples repo, but I don't mind either way.

I'll make a version of this that uses the opposite conventions (as per Google C++ style guide), and you can approve whichever :D

@willgittoes-dd
Copy link
Contributor Author

eyyyy, try this one instead: #103

@lucaspimentel
Copy link
Member

We're going with #103 instead. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:builds project files, build scripts, pipelines, versioning, releases, packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants