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

Distribute GE PluginManager with default nuget.org package source #50

Closed
maraf opened this issue Nov 15, 2018 · 10 comments
Closed

Distribute GE PluginManager with default nuget.org package source #50

maraf opened this issue Nov 15, 2018 · 10 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@maraf
Copy link
Owner

maraf commented Nov 15, 2018

Based on discussion at gitextensions/gitextensions#5775.
Original issue #39.

@maraf maraf added the enhancement New feature or request label Nov 15, 2018
@maraf
Copy link
Owner Author

maraf commented Nov 17, 2018

@RussKie
PluginManager recognizes GE plugins by checking dependencies and looking for the GitExtensions.Plugins.

Are you ok (for a GE community) with me pushing this package to nuget.org?
Once it is there, it couldn't be changed, I would be the owner, etc.

One more idea:
Currently this package is empty, but I was also considering that it could contain GE binaries and plugin could use those as compile time reference.

@maraf maraf added bug Something isn't working and removed enhancement New feature or request labels Nov 17, 2018
@maraf maraf added this to the v0.7.1 milestone Nov 17, 2018
@maraf
Copy link
Owner Author

maraf commented Nov 17, 2018

I'm marking this a as bug.
We can't include NuGet.config in release/package as with every update, it would override user defined config.

When config file doesn't exist, NuGet automatically creates one and adds nuget.org source, so ideal.

@maraf maraf self-assigned this Nov 17, 2018
@RussKie
Copy link

RussKie commented Nov 18, 2018

PluginManager recognizes GE plugins by checking dependencies and looking for the GitExtensions.Plugins.

Could you please provide more context and the vision for this assembly/package - e.g. what it is, where the source is. versioning story etc.
Right now, my attention is stretched thin between RL commitments and the upcoming release, so I apologies for asking more questions that may typically require.
/cc: @spdr870 @gerhardol @drewnoakes

We can't include NuGet.config in release/package as with every update, it would override user defined config.

I don't think we should, but I'd like the plugin manager to have the primary feed entry set to nuget.org instead of myget.org when distributed with GE.

@maraf
Copy link
Owner Author

maraf commented Nov 18, 2018

Could you please provide more context and the vision for this assembly/package - e.g. what it is, where the source is. versioning story etc.

Right now it's only meta-package. There's nothing in it. PluginManager uses this dependency only to recognize packages that are intended as GE plugins. My original idea was that this package version will match with GE versions. Plugins could than say "Hey, I'm compatible with GE >= v3.0.0 & < v4.0.0" (as an example) and PluginManager will respect it. In #40 I have removed this version check after some discussion with you.

So, from PluginManager-v.0.6.0 it only matters that this package exists. Version is ignored.

Than I found it could be useful also in a second way. When developing a plugin, you need GE binaries.
You can point to references to Program Files\GE or you can include GE binaries in your repository (like PluginManager does here).
But what if we place GE binaries in the meta-package GitExtensions.Plugins? PluginManager ignores dependency content, so these won't be extracted during plugin installation, and plugin developer could use this package to compile against.

I don't think we should, but I'd like the plugin manager to have the primary feed entry set to nuget.org instead of myget.org when distributed with GE.

Yes, it will be. It's how package NuGet.Client works.
If NuGet.config doesn't exist, it will create one with nuget.org feed included. It works exactly the way we want.

@RussKie
Copy link

RussKie commented Nov 18, 2018 via email

@gerhardol
Copy link

it would be more appropriate for GE to publish the meta package. Thinking out loud, it probably should contain GitUIPluginInterfaces.dll.

yes

The version dependency will be for the version of GitUIPluginInterfaces.dll as well as some other dlls: GitUI.dll, GitHub*.dll ICSharp, ConEmu as well some external dlls. I guess a few of them can be different in the main app vs the plugins, it will have to separated in GE too.

@RussKie
Copy link

RussKie commented Nov 18, 2018

Given that we are on a release runway and I am overwhelmed with RL commitments, I propose another option for consideration.

We bundle the PM into the upcoming release of GE, but we don't make any announcements about it.
This way we can continue with the plugin management story. We will work through the API, the versioning and developer experience stories past the release.

Thoughts?

@maraf
Copy link
Owner Author

maraf commented Nov 18, 2018

The version dependency will be for the version of GitUIPluginInterfaces.dll as well as some other dlls: GitUI.dll, GitHub*.dll ICSharp, ConEmu as well some external dlls. I guess a few of them can be different in the main app vs the plugins, it will have to separated in GE too.

That's exactly the idea.

We bundle the PM into the upcoming release of GE, but we don't make any announcements about it.
This way we can continue with the plugin management story. We will work through the API, the versioning and developer experience stories past the release.

I'm ok with this.
Maybe the dependency test can pass even when the GE.Plugins package doesn't exist in the feed. I'm going to try...

@maraf
Copy link
Owner Author

maraf commented Nov 19, 2018

PluginManager now points to nuget.org by default.
I have published GitExtensions.PluginManager to nuget.org as well, so further updates can be done through this feed.

Regarding to GE.Plugins packages: Dependency check works even if this package isn't in the feed. It's kind a hacky, but for this (not-announced) release it's IMO ok.

@maraf maraf closed this as completed Nov 19, 2018
@RussKie
Copy link

RussKie commented Nov 19, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants