Replaced default remote repo provider #67
Conversation
BarabanovVVl
commented
Jan 13, 2025
BarabanovVVl
commented
- GitHub API can be choosed using --provider github variable
- Default API - Forgejo
- GenerateCommand can be modified through custom EventProviderGenerator
- GitHub API can be choosed using --provider github variable - Default API - Forgejo - GenerateCommand can be modified through custom EventProviderGenerator
| enum RemoteRepoProviderType: String { | ||
| case forgejo = "forgejo" | ||
| case github = "github" | ||
| case unknown |
There was a problem hiding this comment.
кажется, unknown быть не должно, мы с ним работать не можем
| case github = "github" | ||
| case unknown | ||
|
|
||
| static func from(string: String) -> Self { |
There was a problem hiding this comment.
Вот здесь лучше кидать ошибку, если невалидный string, то есть метод будет throws
| case unknown | ||
|
|
||
| static func from(string: String) -> Self { | ||
| switch string.lowercased() { |
There was a problem hiding this comment.
А можно же просто?
RemoteRepoProviderType(rawValue: string.lowercased())| case forgejo = "forgejo" | ||
| case github = "github" |
There was a problem hiding this comment.
Как будто бы можно оставить просто:
case forgejo
case github| let repoProviderType = RemoteRepoProviderType.from(string: provider) | ||
|
|
||
| var remoteRepoProvider: RemoteRepoProvider? | ||
|
|
||
| switch repoProviderType { | ||
| case .forgejo: | ||
| remoteRepoProvider = ForgejoRemoteRepoProvider(httpService: httpService) | ||
| case .github: | ||
| remoteRepoProvider = GitHubRemoteRepoProvider(httpService: httpService) | ||
| case .unknown: | ||
| throw DependeciesGeneratorError.unknownProvider | ||
| } | ||
|
|
||
| guard let remoteRepoProvider = remoteRepoProvider else { | ||
| throw DependeciesGeneratorError.unknownProvider | ||
| } |
There was a problem hiding this comment.
С учетом предыдущих комментов здесь будет примерно так:
let repoProviderType = try RemoteReportProviderType.from(string: provider)
let remoteReporProvider = switch repoProviderType {
case .forgejo:
ForgejoRemoteRepoProvider(httpService: httpService)
case .github:
GitHubRemoteRepoProvider(httpService: httpService)
}|
|
||
| // MARK: - Initializers | ||
|
|
||
| init(from decoder: Decoder) throws { | ||
| if let container = try? decoder.container(keyedBy: CodingKeys.self) { | ||
| self = .gitHub(configuration: try container.decode(forKey: .gitHub)) | ||
| //TODO: - |
| // | ||
| // RemoteRepoCommit.swift | ||
| // AnalyticsGen | ||
| // | ||
| // Created by v.barabanov on 10.01.2025. | ||
| // |
There was a problem hiding this comment.
Хидеры в файлах не нужны, обычно удаляем
There was a problem hiding this comment.
В других новых файлах тоже стоит убрать
There was a problem hiding this comment.
автоматом создалось и не удалил) почищу
| // Created by v.barabanov on 13.01.2025. | ||
| // | ||
|
|
||
| struct QueryCount: Encodable { |
There was a problem hiding this comment.
Не стоит ли добавить префикс Forgejo, если это относится только к нему? Или вынести из папки Forgejo
| struct ForgejoRemoteRepoProvider: RemoteRepoProvider { | ||
|
|
||
| // MARK: - Instance Properties | ||
| private let baseURL = URL(string: "https://forgejo.pyn.ru/api/v1")! |
There was a problem hiding this comment.
Сложно ли будет forgejo.pyn.ru передавать через настройки/параметры? Если да, давай оставим TODO, хотелось бы сделать универсальным
There was a problem hiding this comment.
не упремся снова в то, что от разработчиков потребуется передавать доп переменную окружения?
Можно как вариант перенести в yaml настройки эту функцию.
Отдельный вопрос: в чем тут выгода? если для каждого провайдера свои методы скорее всего (единствкенный плюс переключения на src.pyn а нужен ли?)
There was a problem hiding this comment.
если для каждого провайдера свои методы скорее всего
Нет, у Forgejo единый API же, может отличаться только от версии сервера
There was a problem hiding this comment.
Вот тут и вопрос, вроде как обсудили что src.pyn не нужен будет.
Энивэй, давай сделаю)
|
|
||
| // MARK: - Instance Properties | ||
| private let baseURL = URL(string: "https://forgejo.pyn.ru/api/v1")! | ||
|
|
There was a problem hiding this comment.
Пустая строка, надо бы SwiftLint подключить
There was a problem hiding this comment.
Его скорее всего настроить нужно
| func createGenerator(for provider: String, remoteHost: String) throws -> EventGenerator { | ||
|
|
||
| guard let baseURL = URL(string: remoteHost) else { | ||
| throw DependeciesGeneratorError.missingRemoteHostURI |
There was a problem hiding this comment.
Скорее не missing, а не валидный URL?
| case .unknownProvider: | ||
| "The specified remote repository provider is unknown" | ||
| case .missingRemoteHostURI: | ||
| "The remote repository URI is not specified or incorrect" |
There was a problem hiding this comment.
is not specified
По идее он должен быть дефолтный, если его не указали
