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.

Sign up for GitHub

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

Jump to bottom

fixed supported code for clang. fix build options for macos system #62

Merged
merged 2 commits into from
Jul 13, 2022
Merged

fixed supported code for clang. fix build options for macos system #62

merged 2 commits into from
Jul 13, 2022

Conversation

kmilo9999
Copy link
Contributor

  • Fixed code to supres c++ clang errors.
  • Fixed cmake scripts for macos and choreograph for clang

@kmilo9999 kmilo9999 requested a review from RobertGemmaJr July 6, 2022 20:15
RobertGemmaJr
RobertGemmaJr requested changes Jul 11, 2022
View reviewed changes
Copy link
Member

@RobertGemmaJr RobertGemmaJr left a comment

Choose a reason for hiding this comment

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

Just needs a run though the linter and a couple additions to the README I think. Looking good! This is the perfect size for a PR, nice work

libs/Texture/Texture.cpp Outdated
Comment on lines 54 to 55
#if (!defined(__APPLE__))
if (paths.size() == 0)
Copy link
Member

Choose a reason for hiding this comment

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

I'd run everything through the linter again to fix the tabbing here. Is it usually #if or #IF? Whatever's best practice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no rule about preprocesor directives. The convention is to CAPS the name of the macros. Let's leave #if.
I ran the formatter.

src/interaction/Simulation.cpp
Comment on lines +66 to +71
#if (!defined(__APPLE__))
if (m_controller_app.get_movie_state() == MOVIE_RECORD)
{
m_controller_app.stop_movie();
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Are you essentially turning off some features that don't work on Mac? I would indicate any OS differences in the README

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. I'll add this to the readme.

@kmilo9999 kmilo9999 requested a review from RobertGemmaJr July 11, 2022 17:06
RobertGemmaJr
RobertGemmaJr approved these changes Jul 12, 2022
View reviewed changes
Copy link
Member

@RobertGemmaJr RobertGemmaJr left a comment

Choose a reason for hiding this comment

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

Awesome! Just needs the notes in the readme

@kmilo9999
Copy link
Contributor Author

I added as part of #51

@kmilo9999 kmilo9999 merged commit ed6e2aa into master Jul 13, 2022
@kmilo9999 kmilo9999 deleted the fix-mac-build branch July 13, 2022 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RobertGemmaJr RobertGemmaJr RobertGemmaJr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kmilo9999 @RobertGemmaJr