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

just addressing some compiler warnings on windows #1954

Merged
merged 8 commits into from
Jun 3, 2024

Conversation

IamSanjid
Copy link
Contributor

Describe your changes

Issue ticket number and link

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

@IamSanjid IamSanjid changed the title fixed warning C4244 for UIWebViewImpl-win32 while changing LPWSTR to string just addressing some compiler warnings on windows Jun 3, 2024
@rh101
Copy link
Contributor

rh101 commented Jun 3, 2024

@IamSanjid It would be better if you do all the changes, then create the PR, but since you have already created the PR, then do all the changes and then commit in one go. The reason is that now you've created the PR, every commit you make triggers the build system, or stops it and restarts it, and also sends out email notifications for every single commit to anyone who automatically subscribed to changes in this repo.

So, at the very least, please change this PR to a draft until it is ready to be reviewed.

Please be aware of this for any future changes. It is always best to create the branch on your local PC, make any changes and commit them locally, and when it's complete, then create the pull request against the repository.

@IamSanjid
Copy link
Contributor Author

@IamSanjid It would be better if you do all the changes, then create the PR, but since you have already created the PR, then do all the changes and then commit in one go. The reason is that now you've created the PR, every commit you make triggers the build system, or stops it and restarts it, and then sends out email notifications for every single commit to anyone who automatically subscribed to changes in this repo.

So, at the very least, please change this PR to a draft until it is ready to be reviewed.

Please be aware of this for any future changes. It is always best to create the branch on your local PC, make any changes and commit them locally, and when it's complete, then create the pull request against the repository.

Ow I am sorry I didn't realize that

@IamSanjid IamSanjid marked this pull request as draft June 3, 2024 03:08
@IamSanjid
Copy link
Contributor Author

IamSanjid commented Jun 3, 2024

This should be it for now except regarding 3rd party, lua_bindings and unitialized member warning

@IamSanjid IamSanjid marked this pull request as ready for review June 3, 2024 05:21
@halx99 halx99 merged commit 22ecef1 into axmolengine:dev Jun 3, 2024
29 checks passed
@halx99 halx99 added this to the 2.1.4 milestone Jun 3, 2024
@IamSanjid IamSanjid deleted the fix-c4244-uiweb-win32 branch June 3, 2024 21:25
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