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

improve mingw compatibility and free the console in additional places #588

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

sudomgamal
Copy link
Contributor

@sudomgamal sudomgamal commented Nov 21, 2024

  • the flags added using add_defnintions will be appended to all compiler tools
    including the resource compiler (rc) which breaks the build. Use CMAKE_CXX_FLAGS instead.
  • close the windows console when the user opens the dlt-viewer using a file doubleclick in the windows explorer

@sudomgamal sudomgamal changed the title add fixes for mingw compatibility and free the console in additional … improve mingw compatibility and free the console in additional places Nov 22, 2024
@alexmucde alexmucde added this to the Release v2.28.0 milestone Dec 6, 2024
@alexmucde
Copy link
Collaborator

@sudomgamal Please fix the merge conflict.

@alexmucde
Copy link
Collaborator

Please also add a signed off statement to the commit message.

…places

- the flags added using add_defnintions will be appended to all compiler
  tools including the resource compiler (rc) which breaks the build
  use CMAKE_CXX_FLAGS instead.

- close the windows consolewhen the user opens the dlt-viewer using a file
  doubleclick in the windows explorer

Signed-off-by: sudomgamal <[email protected]>
Signed-off-by: sudomgamal <[email protected]>
@sudomgamal
Copy link
Contributor Author

done

@alexmucde alexmucde requested a review from vifactor January 7, 2025 13:13
@alexmucde
Copy link
Collaborator

@vifactor Please review the change, if possible.

@alexmucde alexmucde added the merge label Jan 7, 2025

if(closeConsole)
{
#if (WIN32 || WIN64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see a code duplicate, since exactly the same piece of code is at the end of this function. Could you please wrap this in a function and use it in two places?

@sudomgamal
Copy link
Contributor Author

done

Copy link
Collaborator

@vifactor vifactor left a comment

Choose a reason for hiding this comment

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

Thanks

@vifactor vifactor merged commit 6eb4b0c into COVESA:master Jan 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants