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

Remove macos-latest job matrix from Test CI #532

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

hasansezertasan
Copy link

@hasansezertasan hasansezertasan commented Feb 5, 2024

Remove macos-latest job matrix from Test CI

As in the actions/setup-python#860, some Python versions aren't available in the macos-latest os.

@stevepiercy
Copy link
Member

@hasansezertasan thank you for your contribution. I need to find why the lint test fails. I suspect it is a change in the latest version of Black. I will need to make time to dig into it, but if you have any information that can help, I would appreciate it and perhaps get this merged faster. Please let me know. Thank you!

@hasansezertasan
Copy link
Author

@hasansezertasan thank you for your contribution. I need to find why the lint test fails. I suspect it is a change in the latest version of Black. I will need to make time to dig into it, but if you have any information that can help, I would appreciate it and perhaps get this merged faster. Please let me know. Thank you!

I already tried to find the reason why the lint test failed but couldn't, so I don't have any information about that. Good luck! 🙏

You are most welcome 😇.

@hasansezertasan
Copy link
Author

Hey @stevepiercy, do you have any progress on the issue?

@stevepiercy
Copy link
Member

I no longer use deform in any of my client projects. I've pinged the other maintainers here @miohtama @mcdonc @ericof to see if they have any interest in taking it on.

We could also take on new maintainers. Would you be interested?

@hasansezertasan
Copy link
Author

I no longer use deform in any of my client projects. I've pinged the other maintainers here @miohtama @mcdonc @ericof to see if they have any interest in taking it on.

I see, it is very sad to hear this.

We could also take on new maintainers. Would you be interested?

I might be but I don't think I'm there yet 🥺. Can you please re-run the workflow so I can check out what's wrong 🤓?

@stevepiercy
Copy link
Member

@hasansezertasan GitHub lacks a way to rerun a workflow whose logs have expired. I'm going to close and reopen this PR to see if that jiggers the switch. If not, then you can push a commit to your branch, and that should trigger the build, or at least let me know that I need to approve a run.

@stevepiercy stevepiercy closed this Jun 8, 2024
@stevepiercy stevepiercy reopened this Jun 8, 2024
@stevepiercy
Copy link
Member

Yup, closing and reopening this PR did the trick. @hasansezertasan CI results should be available in a few minutes.

@hasansezertasan
Copy link
Author

hasansezertasan commented Jun 8, 2024

Yup, closing and reopening this PR did the trick. @hasansezertasan CI results should be available in a few minutes.

Thank you for your help 🙏.

@hasansezertasan
Copy link
Author

I seemed to pass the Build and test / Lint the package job but there are new errors in different jobs.

@hasansezertasan
Copy link
Author

I tried to bump the setup-python action but it didn't work.

Here is a related issue: actions/setup-python#860

@hasansezertasan
Copy link
Author

I don't think Pylons/hypatia#19 is the best solution... What do you think we should do here @stevepiercy?

@stevepiercy
Copy link
Member

@hasansezertasan why do think Pylons/hypatia#19 is not the best solution? What would be better? I have no idea. @Pylons/deform-colander-developers maybe y'all can weigh in?

@hasansezertasan
Copy link
Author

@hasansezertasan why do think Pylons/hypatia#19 is not the best solution? What would be better? I have no idea. @Pylons/deform-colander-developers maybe y'all can weigh in?

I rarely run my CI tests on macos-latest, so I don't know the reasoning behind running these tests on macos-latest. Finally, I won't be able to tell if it's a good approach. Ultimately, we are skipping tests for four Python versions on macos-latest.

If this approach is acceptable, there should be a reminder that this workaround should be fixed as soon as actions/setup-python#860 gets fixed.


One more thing, if pylons devs decide to accept this CI fix, I should open another PR for translations and remove translations from this PR.

.github/workflows/ci-tests.yml Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Member

I rarely run my CI tests on macos-latest, so I don't know the reasoning behind running these tests on macos-latest.

Many developers who use Deform in their apps use macOS on their system while developing. The macOS CI runs ensure that Deform will work on those systems.

Finally, I won't be able to tell if it's a good approach. Ultimately, we are skipping tests for four Python versions on macos-latest.

We can only test on what is available at the time. Agreed that it is unfortunate not to test on Python 3.11 or 3.12, but when GitHub fixes the issue, we can add it. I don't think pypy-3.10 ever worked on macos-latest.

At this time, we want to run CI on Python 3.8 - 3.12 and PyPy 3.10, whenever possible.

If this approach is acceptable, there should be a reminder that this workaround should be fixed as soon as actions/setup-python#860 gets fixed.

Yes, please correct a new issue for this item.

One more thing, if pylons devs decide to accept this CI fix, I should open another PR for translations and remove translations from this PR.

Agreed.

Thank you for your diligence! We really appreciate your efforts.

@tseaver I'd appreciate your review of this PR when it is ready.

@stevepiercy stevepiercy requested a review from tseaver June 8, 2024 21:51
@hasansezertasan
Copy link
Author

Many developers who use Deform in their apps use macOS on their system while developing. The macOS CI runs ensure that Deform will work on those systems.

Well, we just deleted ci test for macos-latest 😆.

Thank you for your diligence! We really appreciate your efforts.

You're welcome 🤗. I'll update the contents of this PR and rename it. I'll let you know when it's done 👍.

@hasansezertasan hasansezertasan changed the title Added Turkish translation Remove masoc-latest job matrix from Test CI Jun 8, 2024
@hasansezertasan hasansezertasan changed the title Remove masoc-latest job matrix from Test CI Remove macos-latest job matrix from Test CI Jun 8, 2024
@hasansezertasan
Copy link
Author

@stevepiercy, I think it's ready to review. I'm not sure if the title follows the right convention for you people, so feel free to update it as you want 🚀.

@stevepiercy
Copy link
Member

@hasansezertasan would you please take a look at @digitalresistor's recent changes to Waitress for CI, updated yesterday? They figured out several issues that will help with this PR.

https://github.com/Pylons/waitress/pull/440/files

Not everything needs to be ported over, and some stuff may need to be adapted. Thank you!

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