Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support Net6 target for DynamoCoreTests.dll (WIP for feedback) #13766
Support Net6 target for DynamoCoreTests.dll (WIP for feedback) #13766
Changes from 40 commits
4daf4dd
fcc572e
de39a63
802367b
2258630
7377cdb
7e66e50
4ee22ff
e1a5e7e
22435c3
24ffb90
d1b5e73
a2df0ca
cc08b3a
d7e1abf
46ec462
2f0a375
8f6ed83
abb1490
48ee12e
c296bbf
e919593
352ee50
53dc2d5
05c43a3
2543b1c
a95f53e
e04d5e2
40c24b6
b8a6f62
5ec9b2e
6a2fde2
7b46b2f
f03fe35
4b2fff7
e21c743
03b0602
1695362
bceedca
d2a5e47
9d8c90c
ac55692
1e0d11b
8bd66fc
67fe4c3
4243d4a
a842eb1
dc882b3
2f8ecfd
aaf1061
3fbbac0
25e1919
77207e1
767ac54
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot tell, but did you add dsoffice to the Linux part of the solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it for NET6 in general - so yes. DSOffice also includes openxml and CSV nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh..the Moq dependency.
I've tried before to solve the issue of test dependencies in the final output path but I could not find a one size fits all solution.
Maybe the easiest way is to put everything in there, then just filter out (but I know some file escaped in the past and we got stuck with them)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I mention a few in the pr description - I don't intend to solve it here, but I don't want to make it worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up continuing with your approach -
Moq
's new deps are copied to the test_dependencies folder. In additionMoq
is also copied to the bin folder since we cannot remove it until a GL release.I've also added a bunch of
SetupFixtures
that hook up the resolver you added for other test assemblies that started failing now that the new moq deps are not in the bin folder.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand why we would avoid Excel on linux but why are we limiting it to netframework only ?
Does it not work on dotnet6+ ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is something I tried before
https://github.com/DynamoDS/Dynamo/pull/12716/files#diff-a9d51e372ddc4731d6c88a4bfbb2dcd01b5dfa880e0d34d34078f09fd4aaf9a7R30
I think I also added a note about this on the wiki
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.nuget.org/packages/Microsoft.Office.Interop.Excel#supportedframeworks-body-tab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will leave this for another time and file a followup if thats ok.