Implement support for Gitea
This is the PR to track the development of support for Gitea.
Description
It adds support specifically for Gitea by implementing GiteaProvider (an IVcsProvider descendant) but also expands the main code by adding capabilities to choose between and configure custom providers.
Related Issue
Issue #252
Motivation and Context
See linked issue
How Has This Been Tested?
It's currently manually tested against https://try.gitea.io which is a free and publicly available sandbox instance of Gitea.
Screenshots (if appropriate):
Checklist:
- [ ] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
The client API is generated using the latest official API schema
The tool I've used is openapi-generator and the resulting code is in a separate repo which is currently locally referenced in the project.
The plan is to fix all the issues in the client API (the generator is far from being perfect) and when it's done (or at least good enough for use with GRM) publish it on NuGet and replace all the references to the public package.
This pull request introduces 64 alerts when merging 68c2fc21bc2fb948cab4db51a81ec05dd7423d48 into 1c032346739e85e2d36b1bdfbd68c26217b81189 - view on LGTM.com
new alerts:
- 64 for Use of default ToString()
@belidzs having the IVcsProvider is still likely to be needed. @akordowski has expressed an interest in adding DI to the project for other reason, and we are likely going to need that interface to allow this to happen. For what I think you want to do, I would suggest both an interface, and an abstract base class (with virtual methods). That way, we can provide default implementations of the internal methods, but which can be overridden when required.
Does that make sense?
Would be good to get some thoughts from @AdmiringWorm and @akordowski here.
@gep13 you're right it makes sense to use interfaces for that. I've recently started using DI in .NET Core and I really love the concept.
I've already started extracting some common stuff into the abstract class, however I have a feeling that probably I should use AutoMapper and pass complex model objects to functions instead of raw properties (e.g. a Model.Issue instead of index), but I'm not sure which one is the better solution.
Can you take a look at a6447c5 and confirm that this is the direction you had in mind regarding the inheritance?
I was more thinking along the lines of:
public class GiteaProvider : VcsProviderBase, IVcsProvider
And then only having the overrideable methods in the VcsProviderBase class.
Wouldn't that be redundant? VcsProviderBase already has to inherit from the interface since it provides a default implementation of CreateLabels()
VcsProviderBase doesn't need to inherit from IVcsProvider. The base class will only contain the methods that were private in the GitHubVcsProvider, with the default implementation, that can then be overridden if/when required.
@belidzs having the IVcsProvider is still likely to be needed. @akordowski has expressed an interest in adding DI to the project for other reason, and we are likely going to need that interface to allow this to happen.
We don't need to have an interface even when implementing DI. A abstract class would be just as valid instead of an interface. I don't care either way if there is an interface or an abstract base class that is being used.
This pull request introduces 64 alerts when merging 293164cb8087137400718026168160173ae00091 into 1c032346739e85e2d36b1bdfbd68c26217b81189 - view on LGTM.com
new alerts:
- 64 for Use of default ToString()
@belidzs I'm currently at the implementaion of the DI. I wouldn't put much effort into the VCsProvider before we have a stable and functioning DI implementaion. So you would avoid to fix the merge conflicts later.
@akordowski thanks for the heads up, I wasn't aware that there was anything else going on so I have already done some heavy refactoring on the GitHubProvider class. I hope I'll be able to escape from the merge hell after you've finished.
Can you let me know if you're done with GitHubProvider and IVcsProvider?
@belidzs I will publish a draft PR the comming days so we can discuss the next step. I will inform you then.
Cheers @akordowski
Is it safe to start working on this again?
Actually, yes. There are some pending PRs so at the end there might be some merge conflicts. But the refactorings for DI are already merged into the develop branch.