Skip to content

Conversation

@zhmt
Copy link
Contributor

@zhmt zhmt commented Feb 7, 2025

It should be ok when folds exist. It will fail to build without this patch, in vs code on windows with compiler ( visual studio community 2022 amd 64) .

@zhmt zhmt requested a review from a team as a code owner February 7, 2025 08:02
@dsnopek
Copy link
Collaborator

dsnopek commented Mar 13, 2025

Thanks!

This seems fine, however, you'll need to fix the code style issue pointed out by Naros to get CI to pass. While you're at it, could you rewrite the commit message to describe the change, maybe something like binding_generator.py: Don't error if directory already exists?

@zhmt
Copy link
Contributor Author

zhmt commented Mar 14, 2025

Thanks!

This seems fine, however, you'll need to fix the code style issue pointed out by Naros to get CI to pass. While you're at it, could you rewrite the commit message to describe the change, maybe something like binding_generator.py: Don't error if directory already exists?

Sorry about this,I am not farmilar with github. I will update it soon.

@zhmt
Copy link
Contributor Author

zhmt commented Mar 14, 2025

Is it OK? Thanks for correction and suggestion.

@enetheru
Copy link
Collaborator

@zhmt you'll need to squash the two commits into one. The current title of the commit message is "Update binding_generator.py" and then goes into detail that will hidden and require interaction of some sort in most git software and here on github, so it's probably best to just copy dsnopek's suggestion verbatim: "binding_generator.py: Don't error if directory already exists"

@zhmt
Copy link
Contributor Author

zhmt commented Mar 14, 2025

@zhmt you'll need to squash the two commits into one. The current title of the commit message is "Update binding_generator.py" and then goes into detail that will hidden and require interaction of some sort in most git software and here on github, so it's probably best to just copy dsnopek's suggestion verbatim: "binding_generator.py: Don't error if directory already exists"

When I squashed the two commits in github desktop, the push button changes to "force push". It show a s confirming dialog with message below when it clicked :
"A force push will rewrite history on origin/master . Any collaborators working on thisbranch will need to reset their own local branch to match the history of the remote."

Is it safe ?

It should be ok when folders exist. Exception shouldn't be thrown.

Update binding_generator.py

It should be ok when folds exist. It will fail to build without this patch,  in vs code on windows with compiler ( visual studio community 2022 amd 64) .

Co-Authored-By: Chris Cranford <[email protected]>
@zhmt
Copy link
Contributor Author

zhmt commented Mar 14, 2025

Commits have been squashed, and title has been changed, should works now.

@dsnopek dsnopek changed the title Update binding_generator.py binding_generator.py: Don't error if directory already exists Mar 14, 2025
@dsnopek dsnopek merged commit 654de13 into godotengine:master Mar 14, 2025
11 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Mar 14, 2025

Thanks! And congrats on your first merged Godot contribution 🎉

@dsnopek
Copy link
Collaborator

dsnopek commented Mar 17, 2025

Cherry-picked for 4.3 in PR #1744

@dsnopek
Copy link
Collaborator

dsnopek commented Mar 17, 2025

Cherry-picked for 4.4 in PR #1745

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.

4 participants