cpan-authors / IPC-Run

https://metacpan.org/pod/IPC::Run
Other
21 stars 38 forks source link

Disable new Windows_Installation workflow. #153

Closed nmisch closed 2 years ago

nmisch commented 2 years ago

For rationale, see comments inside the commit and https://github.com/toddr/IPC-Run/pull/145#issuecomment-852998772.

This makes the test manual-only. I considered using https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule to run it monthly. However, "Notifications for scheduled workflows are sent to the user who last modified the cron syntax in the workflow file."

I'm submitting this in hopes of unblocking the pull requests that got reviews in mid 2021 and have been waiting for someone having repository write access.

atoomic commented 2 years ago

It's a great idea to add workflow_dispatch to the GitHub workflow but please do not disable the mechanism which trigger a workflow on push and pull requests. Thanks

wchristian commented 2 years ago

I think this was mostly stunlocked by way of me hoping mohawk would carry the ball, but him getting distracted. I'm fine to unstuck things and also debug some stuff if you point me at what exactly is bothersome for the tests?

nmisch commented 2 years ago

The test outcome does not react to lib/ or t/ code in the push:

Hence, running this on pushes is a distraction. It's as meaningless as running the IO::Pty test suite on pushes to this (IPC::Run) repository.

If you can spare cycles to make the Windows user experience better, I recommend fixing some bugs that cause disabling of tests under GITHUB_WINDOWS_TESTING. Real users see almost all of those failures, so hiding them from github test runs isn't a long-term solution. Alternately, change the tests to skip in all Windows builds, not just on github. Indeed, those bugs already had GITHUB_WINDOWS_TESTING skippage in the source tree when

138 re-reported them and led to the misdesigned test that I'm disabling here.

wchristian commented 2 years ago

Noah, i must admit reading your posts i often feel like i'm presented with a sphinx puzzle instead of communication meant to be understand by a normal-ass human like me. It would be nice if you could make an effort to speak more directly and with less references. If you must imagine me as an idiot to do that, so be it.

That said, i looked at the file and if i understand this right, the cpanm invocation there is useless as it always installs the latest cpan version.

I do however know that cpanm CAN install from git urls:

For a git repository, you can specify a branch, tag, or commit SHA to build. The default is master

cpanm git://github.com/plack/Plack.git@1.0000        # tag
cpanm git://github.com/plack/Plack.git@devel         # branch

I am however unfamiliar with github workflows. It appears to me the best solution would be to have that workflow changed to use the SHA of the commit that was pushed to trigger it, to instruct CPAN where to install from. Can and would you implement that?

(Also frankly at this point i have no idea what that last paragraph means, and it would be nice if you could link to a bug report in the traditional format of "i do this, this happens, i expect this to happen".)

nmisch commented 2 years ago

Here's the simple explanation for this PR. Windows_Installation doesn't test the IPC/Run.pm being pushed, so it should not run on push.

Yeah, calling cpanm like you show may work. Fixing GITHUB_WINDOWS_TESTING would be more important. I'm not planning either project. If you want to learn about the GITHUB_WINDOWS_TESTING problem, do "git grep GITHUB_WINDOWS_TESTING". Here's what's happening. We disable a bunch of tests when running on github, but all the other Windows users still run those tests and still get failures.

wchristian commented 2 years ago

Awesome, thanks for the clarifications. I'll be looking at that tomorrow. :)