Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
This review identified several critical issues in the Yamato configuration files, specifically the removal of the -a arm64 architecture flag for macOS validation jobs, which could lead to performance regression and incorrect validation. Additionally, potential YAML indentation errors and consistency issues with generated configuration were found in the trigger definitions.
🤖 Helpful? 👍/👎
| - command: pip install semver requests --index-url https://artifactory-slo.bf.unity3d.com/artifactory/api/pypi/pypi/simple | ||
| - command: python PythonScripts/print_machine_info.py | ||
| - command: unity-downloader-cli -u 6000.0/staging -c editor -a arm64 --path .Editor --fast | ||
| - command: unity-downloader-cli -u 6000.0/staging -c editor --path .Editor --fast |
There was a problem hiding this comment.
⚠️ Verification: This finding may be speculative — The finding uses the hedging phrase "will likely cause" regarding the tool's behavior when the architecture flag is omitted.
The -a arm64 flag was removed from the unity-downloader-cli command for all macOS validation jobs. For Unity 6 (6000.x) and trunk versions, this will likely cause the downloader to fetch the Intel (x64) version of the Unity Editor instead of the native Apple Silicon (arm64) version. This regresses test performance by running under Rosetta and fails to validate the native arm64 editor which is the primary target for modern macOS. If these runners are Apple Silicon, you should retain the explicit architecture flag.
| - command: unity-downloader-cli -u 6000.0/staging -c editor --path .Editor --fast | |
| - command: unity-downloader-cli -u 6000.0/staging -c editor -a arm64 --path .Editor --fast |
🤖 Helpful? 👍/👎
| name: Pull Request Trigger | ||
| dependencies: | ||
| - path: .yamato/wrench/validation-jobs.yml#validate_-_probuilder_-_6000_0_-_macos | ||
| - path: .yamato/wrench/validation-jobs.yml#validate_-_probuilder_-_6000_0_-_windows |
There was a problem hiding this comment.
⚠️ Verification: This finding may be speculative — The finding uses multiple hedging phrases such as "It appears", "seem to", and "will likely" when describing the indentation and its impact.
It appears there is an indentation mismatch for the newly added Windows dependencies. In YAML, all items in a list must be at the same indentation level. Comparing line 5 and 6, the new entries seem to have fewer leading spaces than the existing macOS entries. This will likely cause a parsing error in Yamato.
🤖 Helpful? 👍/👎
| name: Pull Request Trigger | ||
| dependencies: | ||
| - path: .yamato/wrench/validation-jobs.yml#validate_-_probuilder_-_6000_0_-_macos | ||
| - path: .yamato/wrench/validation-jobs.yml#validate_-_probuilder_-_6000_0_-_windows |
There was a problem hiding this comment.
⚠️ Verification: This finding may be speculative — The finding uses the hedging word "might" when speculating about the YAML file being overwritten and the job filter's behavior.
You are manually adding specific Windows jobs to the triggers.yml file, but you also updated the generator logic in Triggers.cs (line 25). If Triggers.cs is used to generate this YAML file, your manual changes might be overwritten. Additionally, the broad filter d.JobId.Contains("windows") in C# might include more jobs than the ones you manually listed here. Have you considered using the generation tool to update this file automatically?
🤖 Helpful? 👍/👎
| - command: pip install semver requests --index-url https://artifactory-slo.bf.unity3d.com/artifactory/api/pypi/pypi/simple | ||
| - command: python PythonScripts/print_machine_info.py | ||
| - command: unity-downloader-cli -u trunk -c editor -a arm64 --path .Editor --fast | ||
| - command: unity-downloader-cli -u trunk -c editor --path .Editor --fast |
There was a problem hiding this comment.
⚠️ Verification: This finding may be speculative — The finding uses the hedging word "likely" to describe the necessity of restoring the architecture flag.
The removal of -a arm64 also affects the trunk validation job. This should likely be restored to ensure native testing on macOS.
| - command: unity-downloader-cli -u trunk -c editor --path .Editor --fast | |
| - command: unity-downloader-cli -u trunk -c editor -a arm64 --path .Editor --fast |
🤖 Helpful? 👍/👎
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## master #665 +/- ##
==========================================
+ Coverage 36.05% 36.98% +0.92%
==========================================
Files 277 278 +1
Lines 34909 35018 +109
==========================================
+ Hits 12588 12951 +363
+ Misses 22321 22067 -254
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Did you run "regenerate.sh"? @lopezt-unity |
2 participants
Purpose of this PR
Updating CI
Links
Jira:
Comments to Reviewers
[List known issues, planned work, provide any extra context for your code.]