Skip to content

Prepare for Windows CI with Github Actions #9595

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

Closed

Conversation

mvorisek
Copy link
Contributor

isolated from #8392 PR, these changes are mostly renames

before merge, please squash into one commit

@iluuu1994
Copy link
Member

Why is moving all these files needed?

@mvorisek
Copy link
Contributor Author

it is not needed, but some CI directories are prefixed with dot, some not, this unifies the file structure

@iluuu1994
Copy link
Member

Then please keep them as is.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mvorisek mvorisek force-pushed the prepare_for_win_gh_actions_testing branch 2 times, most recently from c37298c to c70e625 Compare September 30, 2022 16:32
@mvorisek
Copy link
Contributor Author

Then please keep them as is.

the intention is to separate CI files/directories more which is quite common and helpful for quick orientation in the file structure, would you be fine with prefixing the CI directories (and unifying with .github dir) in master?

@iluuu1994
Copy link
Member

would you be fine with prefixing the CI directories (and unifying with .github dir) in master?

I know Git can sometimes recognize renames but it has still been a pain for me in the past, so I think we should keep this as is.

@mvorisek mvorisek force-pushed the prepare_for_win_gh_actions_testing branch from 707d6cf to 1c588ed Compare September 30, 2022 17:13
@mvorisek mvorisek force-pushed the prepare_for_win_gh_actions_testing branch from bb96c0a to 56110d3 Compare September 30, 2022 21:07
@mvorisek
Copy link
Contributor Author

mvorisek commented Oct 1, 2022

@iluuu1994 the MacOS CI error is unrelated, can you please do a final review and if all ok, please squash and merge

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I'll merge this soon.

@mvorisek
Copy link
Contributor Author

mvorisek commented Oct 7, 2022

@iluuu1994 is soon now? :)

@iluuu1994
Copy link
Member

Merged as b43e494. Note: In PHP-8.2 appveyor/setup_hmailserver.php was added so we might want to move that one too.

@iluuu1994 iluuu1994 closed this Oct 9, 2022
@mvorisek mvorisek deleted the prepare_for_win_gh_actions_testing branch October 9, 2022 16:52
@mvorisek
Copy link
Contributor Author

mvorisek commented Oct 9, 2022

Thank you, I added your comment about hmailserver into #9650.

I would like to add Windows 32 bit testing as well. Almost everything is done, but #9650 is blocking the PR, as it causes like every 3rd CI run or so to fail. Do you have any idea what can be the problem and do you have access to Windows to run like hundred of full testing cycles of x86/32 bit build to reproduce?

@iluuu1994
Copy link
Member

I don't have a Windows machine so I can't help there. Apparently MSVC now has a address sanitizer, you could try that.

https://devblogs.microsoft.com/cppblog/address-sanitizer-for-msvc-now-generally-available/

@cmb69
Copy link
Member

cmb69 commented Oct 9, 2022

Apparently MSVC now has a address sanitizer, you could try that.

See #6964

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