-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add GitExtensions.Extensibility #11431
Conversation
37843bf
to
384919d
Compare
There must be a very huge benefit of moving all these interfaces and utilities to a submodule / NuGet package. |
This will make development of the core app annoying, will slow down the app for the benefit of a few plugins. Most moved functionality does not make sense in a module, for instance GitModule have mostly app internal functions. |
It should not be too difficult to directly publish the GE Extensibility NuGet from the GE main repo, shouldn't it? |
With this, you know if the plugins require changes after a GE update. On the other hand, there are so much core functionality in Extensibility that the plugins will always have to be updated. Adding plugins to an existing app without a designed interface is not smooth. |
I fully support this! But I have concerns / objections to move them out of the repo. I still think my above comments are clear. |
Apologies for the radio silence; the end of last year was very demanding
and draining (mentally and physically). Right now I'm on a vacation with a
very limited access to a computer.
I will respond to the feedback and questions when I'm back in about a
month.
|
4b0708b
to
5f40fa4
Compare
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.
LGTM
I have fixed the first three comments myself.
GitExtensions.Extensibility/Translations/Xliff/TranslationUtil.cs
Outdated
Show resolved
Hide resolved
GitExtensions.Extensibility/Translations/Xliff/TranslationUtil.cs
Outdated
Show resolved
Hide resolved
Move API contracts and definitions to GitExtensions.Extensibility project.
3d825dd
to
23c53b1
Compare
Thank you @mstv |
Proposed changes
The initial move of all shared contracts and helper API to https://github.com/gitextensions/gitextensions.extensibility (see Move API contracts and definitions from GitExtensions repo gitextensions.extensibility#31).Create a new project called Gitextensions.Extensibility and move of all shared contracts and helper API.This change allows for a separation of concerns - shared contracts that affect the main app as well as Git Extensions plugins will now reside in Gitextensions.Extensibility project, and concrete implementations of the contracts reside in the app.
This is a stepping stone in a direction of complete separation of the contracts and future move of those to https://github.com/gitextensions/gitextensions.extensibility project.
I expect further work will be required to clean the contract declarations (e.g., removal of default args as those play tricks on developers) as well as a holistic reassessment of the API surface.
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.