Skip to content

Conversation

Kryptos-FR
Copy link
Member

@Kryptos-FR Kryptos-FR commented May 16, 2025

PR Details

Add CI workflows that will run when PRs are opened, updated and merged.

The workflows can also be triggered manually if necessary.

Results on my fork

image

Notes

Before merging, it would be good to restrict who can run workflows and use the most conservative settings at the beginning. As an example, here what I have on my fork:

image
image
image

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

@Kryptos-FR Kryptos-FR self-assigned this May 16, 2025
@VaclavElias
Copy link
Contributor

Is there any filter that if PR has e.g. README.md update or other files/folders (we could add to the filter later on) then this GitHub Action wouldn't be triggered?

For example this action https://github.com/stride3d/stride-website/blob/master/.github/workflows/stride-website-release-azure.yml has paths-ignore: where I listed files and folders I don't want to trigger the GitHub Action.

@Kryptos-FR Kryptos-FR force-pushed the feature/github-ci branch from 01a3a8c to 033454b Compare May 17, 2025 11:04
@Kryptos-FR
Copy link
Member Author

@VaclavElias I've added paths as suggested. Let me know what you think.

@Kryptos-FR Kryptos-FR requested review from VaclavElias and xen2 May 17, 2025 11:05
@Kryptos-FR
Copy link
Member Author

Kryptos-FR commented May 17, 2025

@xen2 there is one change that requires updating the jobs in TeamCity:

-<BuildProperties>Configuration=Release;NoWarn=1591;DeployExtension=false;StridePlatforms=$([MSBuild]::Escape('$(StridePlatforms)'));StrideGraphicsApiDependentBuildAll=$(StrideGraphicsApiDependentBuildAll)</BuildProperties>
+<BuildProperties>NoWarn=1591;DeployExtension=false;StridePlatforms=$([MSBuild]::Escape('$(StridePlatforms)'));StrideGraphicsApiDependentBuildAll=$(StrideGraphicsApiDependentBuildAll)</BuildProperties>

I have removed Configuration=Release in BuildProperties so that this parameter can be passed to the job. This means it needs to be added back to all jobs on TeamCity that require a Release build (esp. the ones doing the final packaging). It's ok to change them now: it won't have any effect when running on a branch that stills has the previous value of BuildProperties.

edit: I've changed my mind. The new build jobs will not use Stride.build at all, as that file was mainly created to simplify the process in TeamCity. Instead the actual msbuild and dotnet command lines will be used.

@Kryptos-FR Kryptos-FR force-pushed the feature/github-ci branch from 374fb25 to dbee57b Compare May 20, 2025 13:53
@Kryptos-FR
Copy link
Member Author

@VaclavElias @xen2 as far as I'm concerned the PR is ready. The only remaining task before merging is the configuration on the repository itself. Let me know when that part is done so I can merge.

@xen2
Copy link
Member

xen2 commented May 20, 2025

I have updated action permissions (I checked each of them and ended up with same choices as yours).

@Kryptos-FR Kryptos-FR merged commit 03f6029 into stride3d:master May 20, 2025
2 checks passed
@Kryptos-FR Kryptos-FR deleted the feature/github-ci branch May 20, 2025 15:49
@Kryptos-FR Kryptos-FR mentioned this pull request May 21, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants