DYN-7337 Make TuneUp an Built-In package, part II #15536

QilongTang · 2024-10-09T20:34:08Z

Purpose

Update TuneUp after build copy step so that TuneUp is setup more like a package instead of a OOTB view extension.
Remove the mandate for Built-in package to inlcude node library, remove such warning logging from Don't throw if a package has dlls but no node libraries in a protected location #13625
Investigate the dyf folder empty warning, a dyf folder is not expected for TuneUp
Investigate the loading sequence when the same package exist locally
Investigate the diff between folder name for Built-In package
Clean up console logging

Before:

After:

Expected folder structure under DynamoCore:

Common Errors:

When user have both Built-in package and downloaded package exist
When user tries to download the same package while Built-In package exist
When user tries to download a newer version of the package while Built-In package exist

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated
This PR contains no files larger than 50 MB

Release Notes

Make TuneUp an Built-In package and follow package folder structure, also improve Dynamo logging

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

github-actions

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7337

github-actions · 2024-10-10T14:29:25Z

UI Smoke Tests

Test: success. 11 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests

QilongTang · 2024-10-11T13:49:01Z

src/DynamoCoreWpf/DynamoCoreWpf.csproj

    </ItemGroup>
    <MakeDir Directories="$(OutputPath)\viewExtensions\" />
-    <Copy SourceFiles="@(ExternTuneUpDll)" DestinationFolder="$(OutputPath)\Built-In Packages" />
-    <Copy SourceFiles="@(ExternTuneUpManifest)" DestinationFolder="$(OutputPath)\viewExtensions" />
+    <Copy SourceFiles="@(ExternTuneUpPkg)" DestinationFolder="$(OutputPath)\Built-In Packages\packages\TuneUp\%(RecursiveDir)" />


Copy files recursively and keeping the folder structure

QilongTang · 2024-10-11T13:50:05Z

src/DynamoPackages/PackageLoader.cs

-                Log(String.Format(
-                    String.Format(Resources.InvalidPackageNoNodeLibrariesDefinedInPackageJson,
-                    discoveredPkg.Name, discoveredPkg.RootDirectory)));
-            }


Remove the node library dll check because Built-In package moving forward may not always include nodes so this is no longer needed, even the logging

QilongTang · 2024-10-11T17:59:06Z

.gitignore

+AnyCPU/
+Debug/
+Release/
+originalBinaries/


ignore dlls under built directory more precisely

QilongTang · 2024-10-14T13:35:09Z

Not sure if the two remaining tests about Node Autocomplete is related to this PR at all, re-running in https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/16549/

extern/TuneUp/pkg.json

reddyashish · 2024-10-14T15:51:55Z

tools/DynamoPackagesAnalyzer/DynamoPackagesAnalyzer.csproj

@@ -10,7 +10,7 @@
  <ItemGroup>
    <PackageReference Include="CommandLineParser" Version="2.8.0" />
    <PackageReference Include="CsvHelper" Version="30.0.1" />
-    <PackageReference Include="Greg" Version="3.0.1.4707" />
+    <PackageReference Include="Greg" Version="3.0.2.6284" />


Why are we changing the greg version only for this project?

@reddyashish This project seems to be behind (not aligned with all the other project)

reddyashish · 2024-10-14T15:52:26Z

test/pkgs/subPackageDirectory/runtimeGeneratedExtension/bin/TestViewExtension.dll

Add this dll to the list of shipped binaries?

This is a test dll generated runtime, I have removed it and updated gitignore

RobertGlobant20

LGTM

QilongTang · 2024-10-14T21:53:05Z

@reddyashish @zeusongit I updated to latest TuneUp, and all regressions are passing from previous run so this should be all good

QilongTang · 2024-10-14T22:08:23Z

I dont think the error from the Build DynamoAll.sln is from this branch..

zeusongit · 2024-10-14T22:52:44Z

I dont think the error from the Build DynamoAll.sln is from this branch..

Interestingly, I was getting the same error

zeusongit · 2024-10-14T22:54:19Z

@reddyashish @zeusongit I updated to latest TuneUp, and all regressions are passing from previous run so this should be all good

One test failing, did it pass in another build?

QilongTang · 2024-10-14T23:18:29Z

@reddyashish @zeusongit I updated to latest TuneUp, and all regressions are passing from previous run so this should be all good

One test failing, did it pass in another build?

@zeusongit Yup, the reported failure is a sporadic one. The build errors is from master branch itself, most likely from #15439. I dont think we are supposed to merge that..

update copy location

bd362c2

github-actions bot reviewed Oct 9, 2024

View reviewed changes

QilongTang added 2 commits October 9, 2024 16:49

Updates

3fd7dc3

Update

e41dcb5

QilongTang added this to the 3.4 milestone Oct 9, 2024

QilongTang added 4 commits October 9, 2024 17:34

Update

9ab1f81

Update

9326858

Update

610dd6c

add time unit

d543528

QilongTang marked this pull request as ready for review October 10, 2024 14:26

QilongTang changed the title ~~DYN-7337 Make TuneUp an OOTB package, part II~~ DYN-7337 Make TuneUp an Built-In package, part II Oct 10, 2024

QilongTang added 4 commits October 10, 2024 15:13

Update

909be15

add more logging

a4e10b2

remove the mandate for Built-In package to include node library

519d66e

Skip loading custom nodes if dyf folder does not exist

d9e868b

QilongTang commented Oct 11, 2024

View reviewed changes

QilongTang added 2 commits October 11, 2024 09:51

clean up

716e7dd

Touch gitignore to be more precise

9410efb

QilongTang commented Oct 11, 2024

View reviewed changes

.gitignore

AnyCPU/

Debug/

Release/

originalBinaries/

Copy link

Contributor Author

QilongTang Oct 11, 2024

Choose a reason for hiding this comment

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

ignore dlls under built directory more precisely

fix unit test

781f3c2

QilongTang requested review from zeusongit, reddyashish and RobertGlobant20 October 14, 2024 01:01

Update unit tests with new baseline

695d351

reddyashish reviewed Oct 14, 2024

View reviewed changes

extern/TuneUp/pkg.json Outdated Show resolved Hide resolved

reddyashish reviewed Oct 14, 2024

View reviewed changes

QilongTang added 3 commits October 14, 2024 11:34

Update unit test

d10c25c

delete dll

d55c83c

Update gitignore

2b773bb

RobertGlobant20 approved these changes Oct 14, 2024

View reviewed changes

QilongTang requested a review from reddyashish October 14, 2024 18:50

Update TuneUp to v1.0.13

858ca4b

QilongTang mentioned this pull request Oct 14, 2024

new wizzard control #15439

Merged

9 tasks

reddyashish approved these changes Oct 15, 2024

View reviewed changes

QilongTang merged commit 4d498ff into master Oct 15, 2024
23 of 25 checks passed

QilongTang deleted the TuneUpRefine branch October 15, 2024 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DYN-7337 Make TuneUp an Built-In package, part II #15536

DYN-7337 Make TuneUp an Built-In package, part II #15536

QilongTang commented Oct 9, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot commented Oct 10, 2024 •

edited

Loading

QilongTang Oct 11, 2024 •

edited

Loading

QilongTang Oct 11, 2024

QilongTang Oct 11, 2024

QilongTang commented Oct 14, 2024

reddyashish Oct 14, 2024

QilongTang Oct 14, 2024

reddyashish Oct 14, 2024

QilongTang Oct 14, 2024

RobertGlobant20 left a comment

QilongTang commented Oct 14, 2024

QilongTang commented Oct 14, 2024

zeusongit commented Oct 14, 2024

zeusongit commented Oct 14, 2024

QilongTang commented Oct 14, 2024 •

edited

Loading

DYN-7337 Make TuneUp an Built-In package, part II #15536

DYN-7337 Make TuneUp an Built-In package, part II #15536

Conversation

QilongTang commented Oct 9, 2024 • edited Loading

Purpose

Declarations

Release Notes

Reviewers

FYIs

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2024 • edited Loading

UI Smoke Tests

QilongTang Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

QilongTang Oct 11, 2024

Choose a reason for hiding this comment

QilongTang Oct 11, 2024

Choose a reason for hiding this comment

QilongTang commented Oct 14, 2024

reddyashish Oct 14, 2024

Choose a reason for hiding this comment

QilongTang Oct 14, 2024

Choose a reason for hiding this comment

reddyashish Oct 14, 2024

Choose a reason for hiding this comment

QilongTang Oct 14, 2024

Choose a reason for hiding this comment

RobertGlobant20 left a comment

Choose a reason for hiding this comment

QilongTang commented Oct 14, 2024

QilongTang commented Oct 14, 2024

zeusongit commented Oct 14, 2024

zeusongit commented Oct 14, 2024

QilongTang commented Oct 14, 2024 • edited Loading

QilongTang commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

QilongTang Oct 11, 2024 •

edited

Loading

QilongTang commented Oct 14, 2024 •

edited

Loading