Skip to content

ci: downgrade target unity version and remove windows platform target#443

Closed
0xFA11 wants to merge 4 commits intodevelopfrom
downgrade-ci
Closed

ci: downgrade target unity version and remove windows platform target#443
0xFA11 wants to merge 4 commits intodevelopfrom
downgrade-ci

Conversation

@0xFA11
Copy link
Copy Markdown
Contributor

@0xFA11 0xFA11 commented Jan 14, 2021

As per @JesseOlmer's suggestion, this PR is an attempt to downgrade MLAPI package targeting Unity 2019.4 and removing Windows platform target temporarily. An ILPP issue on 2019.4 and 2020.1 that's specific to Windows platform is holding us back but while working on that issue, we might be able to keep working on 2019.4 target.

{
using PooledBitStream copy = PooledBitStream.Get();
do
using (var copy = PooledBitStream.Get())
Copy link
Copy Markdown
Contributor Author

@0xFA11 0xFA11 Jan 14, 2021

Choose a reason for hiding this comment

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

this using var declaration is C# 8.0 which is not supported on 2019.4 and 2020.1 @jeffreyrainy :)
but using (...) { ... } block is fine!

Comment thread .yamato/project.metafile
# for validation
test_editors:
- 2020.2
- 2019.4
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Target both? Or 2019.4, 2021.1, & trunk?

Copy link
Copy Markdown
Contributor Author

@0xFA11 0xFA11 Jan 14, 2021

Choose a reason for hiding this comment

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

just double-checking: 2020.1 or 2021.1?

Comment thread .yamato/project.metafile
# Platforms that will be tested. The first entry in this array will also
# be used for validation
test_platforms:
- name: win
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd either remove the platform from develop_pull_request_trigger target in project-test.yml OR just leave it there but change the github gates to not require that check to pass for PR merges. Doing it the way you have in this PR makes me nervous about losing coverage for newer versions on win platform.

"hideInEditor": false,
"dependencies": {
"com.unity.nuget.mono-cecil": "1.10.1-preview.1",
"com.unity.collections": "0.14.0-preview.16"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to have a conversation about this one ASAP, but O.K. for now.

@0xFA11
Copy link
Copy Markdown
Contributor Author

0xFA11 commented Jan 19, 2021

This PR is no longer needed — PR #447 supersedes this.

@0xFA11 0xFA11 closed this Jan 19, 2021
@0xFA11 0xFA11 deleted the downgrade-ci branch March 5, 2021 17:48
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.

2 participants