Expanded build actions for integration testing on pull requests

maiself / godot-python-extension

Python language bindings for the Godot game engine

https://godot-python-extension.readthedocs.io

MIT License

18 stars 3 forks source link

Expanded build actions for integration testing on pull requests #10

Closed fire closed 1 month ago

fire commented 1 month ago

Testing.

fire commented 1 month ago

~~Wait for my build to complete before merging.~~

fire commented 1 month ago

@maiself can you take a look at this?

hemebond commented 1 month ago

Might want to revert all the indentation changes.

maiself commented 1 month ago

Might want to revert all the indentation changes.

Yeah, this was the first thing I noticed when I took a quick glance thru. I'd appreciate keeping things tab indented for accessibility reasons (as well as consistency).

Been a bit busy today, I'm hoping to finish things up quick so I can have a proper look, but think I'm excited to see this in. I have a few questions from the quick look I did have.

on: pull_request, will it also trigger on push? Will manual trigger still be available?
# archive to preserve execute permission on library files, this was due to a false assumption on my part, turns out Linux doesn't require execute permission on library files, so that bit isn't actually needed...
What does this mean? "Allow lower than python 3.10" Is this the reason for if vs match changes? What platforms is this change for?

Also, big thanks for doing this! :smile:

fire commented 1 month ago

"Allow lower than python 3.10" Is this the reason for if vs match changes? What platforms is this change for?

A previous version of the code failed to use the platform python, and it seems like you force python 3.12. I am not sure if this is better, but the match changes seem like a downgrade that didn't break anything.

fire commented 1 month ago

Will manual trigger still be available?

You can see the there's a manual dispatch left.

I'd appreciate keeping things tab indented for accessibility reasons (as well as consistency).

I have no preference. There's bigger fish to fry. I'm pretty sure I ran the default ruff, but I can check.

fire commented 1 month ago

I reverted the changes to the python:

TODO list for the future:

ruff warnings
unused imports
~~match requires 3.10~~

maiself commented 1 month ago

Ok, looked thru, I think I understand everything. There's some deprecation warnings but might be best to save that for later.

If you're good with the state of it, I'm happy to merge. :+1:

maiself commented 1 month ago

Yay, thanks!