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

Expanded build actions for integration testing on pull requests #10

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

fire
Copy link
Contributor

@fire fire commented Jul 21, 2024

Testing.

@fire
Copy link
Contributor Author

fire commented Jul 21, 2024

Wait for my build to complete before merging.

@fire fire force-pushed the auto-actions branch 16 times, most recently from 248abfb to 4972ff9 Compare July 21, 2024 05:40
@fire
Copy link
Contributor Author

fire commented Jul 22, 2024

@maiself can you take a look at this?

@hemebond
Copy link

Might want to revert all the indentation changes.

@maiself
Copy link
Owner

maiself commented Jul 22, 2024

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! 😄

@fire
Copy link
Contributor Author

fire commented Jul 22, 2024

"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
Copy link
Contributor Author

fire commented Jul 22, 2024

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
Copy link
Contributor Author

fire commented Jul 22, 2024

I reverted the changes to the python:

TODO list for the future:

  1. ruff warnings
  2. unused imports
  3. match requires 3.10

@fire fire changed the title Expanded build matrix for cross-platform support Expanded build actions for continious integration testing on pull requests Jul 22, 2024
@fire fire changed the title Expanded build actions for continious integration testing on pull requests Expanded build actions for integration testing on pull requests Jul 22, 2024
@maiself
Copy link
Owner

maiself commented Jul 22, 2024

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. 👍

@maiself maiself merged commit 63a545d into maiself:master Jul 22, 2024
2 checks passed
@maiself
Copy link
Owner

maiself commented Jul 22, 2024

Yay, thanks!

@fire fire deleted the auto-actions branch July 22, 2024 21:19
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