Skip to content

refactor: remove Unity.Collections package dependency#441

Merged
0xFA11 merged 4 commits intodevelopfrom
remove-collections-dep
Jan 21, 2021
Merged

refactor: remove Unity.Collections package dependency#441
0xFA11 merged 4 commits intodevelopfrom
remove-collections-dep

Conversation

@0xFA11
Copy link
Copy Markdown
Contributor

@0xFA11 0xFA11 commented Jan 14, 2021

Currently, we're not using DataStream and Unity.Collections package dependency might hold us back targeting 2019.4 & 2020.x for MLAPI package release.

Having said that, we might consider using NativeArray from UnityEngine.CoreModule and put in stream/reader/writer implementations into MLAPI side separately and independent from Unity.Collections package.

Comment on lines -4 to -6
"references": [
"GUID:e0cd26848372d4e5c891c569017e11f1"
],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is Unity.Collections package's GUID

Copy link
Copy Markdown
Member

@NoelStephensUnity NoelStephensUnity left a comment

Choose a reason for hiding this comment

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

It is sad to see it go. Changes look like it will do the trick!

@fraserhutch2020
Copy link
Copy Markdown

This needs a conversation.

@fraserhutch2020 fraserhutch2020 self-assigned this Jan 14, 2021
@0xFA11 0xFA11 force-pushed the remove-collections-dep branch from eb89cc2 to fd74fd5 Compare January 15, 2021 04:48

namespace MLAPI.Tests
{
public class PlaceholderTests
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.

Did you mean to leave this in? Seems like either we would want to remove it, add some comments on why it's there or actually implement some tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, that was intentional. I commented a note inline indicating that.

Copy link
Copy Markdown
Contributor

@mattwalsh-unity mattwalsh-unity left a comment

Choose a reason for hiding this comment

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

See comments on the testing module

@0xFA11 0xFA11 merged commit d11088c into develop Jan 21, 2021
@0xFA11 0xFA11 deleted the remove-collections-dep 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.

4 participants