Skip to content
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

DYN-6259 #2972

Merged
merged 1 commit into from
Sep 22, 2023
Merged

DYN-6259 #2972

merged 1 commit into from
Sep 22, 2023

Conversation

jesusalvino
Copy link
Contributor

@jesusalvino jesusalvino commented Sep 20, 2023

Purpose

Fixing the bug https://jira.autodesk.com/browse/DYN-6259 excluding passing the Preferences due to it will be handled in the DynamoModel side based on the PathManager info. Follow up PR of DynamoDS/Dynamo#14427

Declarations

Check these if you believe they are true

  • The code base 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
  • Snapshot of UI changes, if any.

Reviewers

@QilongTang
@reddyashish

@QilongTang
Copy link
Contributor

So this is reverting #2933?

@jesusalvino
Copy link
Contributor Author

So this is reverting #2933?

yes

@@ -607,8 +607,7 @@ private static RevitDynamoModel InitializeCoreModel(DynamoRevitCommandData comma
AuthProvider = new RevitOAuth2Provider(new DispatcherSynchronizationContext(Dispatcher.CurrentDispatcher)),
ExternalCommandData = commandData,
UpdateManager = revitUpdateManager,
ProcessMode = isAutomationMode ? TaskProcessMode.Synchronous : TaskProcessMode.Asynchronous,
Preferences = PreferenceSettings.Instance
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does removing this not bring back other issues with Preferences on Dynamo Revit startup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does removing this not bring back other issues with Preferences on Dynamo Revit startup?

No because we are returning the responsibility of management/creation of the preferences to the DynamoModel here DynamoDS/Dynamo#14427, we need both to make them work properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is reverting the code back to https://github.com/DynamoDS/DynamoRevit/blob/RC2.18.1_Revit2024.1/src/DynamoRevit/DynamoRevit.cs#L476. I think it was our mistake that these changes were not needed in the first place

@Mikhinja
Copy link
Collaborator

Mikhinja commented Sep 22, 2023

If testing passes, can we make sure to also merge in branches RC2.19.0_Revit2025 and RC2.19.0_Revit2024.2, please?

@QilongTang QilongTang merged commit 217f4a6 into DynamoDS:master Sep 22, 2023
@QilongTang
Copy link
Contributor

@jesusalvino Would you initiate the cherry-pick?

jesusalvino added a commit to jesusalvino/DynamoRevit that referenced this pull request Sep 22, 2023
jesusalvino added a commit to jesusalvino/DynamoRevit that referenced this pull request Sep 22, 2023
@jesusalvino
Copy link
Contributor Author

@jesusalvino Would you initiate the cherry-pick?

Done:

#2975
#2976

QilongTang pushed a commit that referenced this pull request Sep 22, 2023
QilongTang pushed a commit that referenced this pull request Sep 22, 2023
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.

3 participants