-
Notifications
You must be signed in to change notification settings - Fork 668
Display Python dep info to users on package manager window #11611
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
Conversation
|
The UX implementation and colors LGTM @zeusongit ! |
|
@zeusongit So this PR is about using dependency field. Is there a different PR using the HostDependency implementation for us to compare? |
| //added to this dummy package for representation, to be removed and replace space at the end with newline above | ||
| if (Name == "dronovBIM_v2") | ||
| { | ||
| hostsString += "CPython3"+ Environment.NewLine + "IronPython2"+ Environment.NewLine + "Revit"; |
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.
@QilongTang Here HostsString is derived from host_dependencies that we are using to display dependencies, including python deps.
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.
Avoid string literals wherever possible. Makes it easy to change it.
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.
This wont be part of the final version and will be removed before it is merged
| element.Versions[0].Item1.full_dependency_ids.Add(new Dependency() { name = "CPython", _id = "3.0" }); | ||
| element.Versions[1].Item1.full_dependency_ids.Add(new Dependency() { name = "IronPython", _id = "2.0" }); | ||
| element.Versions[2].Item1.full_dependency_ids.Add(new Dependency() { name = "IronPython", _id = "2.0" }); | ||
| element.Versions[2].Item1.full_dependency_ids.Add(new Dependency() { name = "CPython", _id = "3.0" }); |
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.
We can add another property such as version or vers to record version instead of _id from PMClient class while implementing.
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.
Yes i think version would be a better choice.
|
I would like to conclude this spike, so that we can move further with discussion/implementation on this topic. There are some observations/concerns:
|
src/DynamoCoreWpf/Views/PackageManager/PackageManagerSearchView.xaml
Outdated
Show resolved
Hide resolved
|
@zeusongit just one more comment, forgot to mention it earlier. LGTM. What about the dummy values added in the PR? Will they be removed once the followup changes are made? If yes, please make sure the next task mentions that. |
Yes @reddyashish the dummy values will be removed, if you see the base branch of the PR that is not |

Purpose
This is a separate branch that will include all the visual changes until the backend is wired up, that includes a way to populate package dependency field with Python versions used in that package.