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.

Sign up for GitHub

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

Jump to bottom

fix: remove dependency from go-openapi #82

Merged
merged 1 commit into from
Mar 19, 2021
Merged

fix: remove dependency from go-openapi #82

merged 1 commit into from
Mar 19, 2021

Conversation

Alexey19
Copy link
Contributor

@Alexey19 Alexey19 commented Mar 9, 2021

No description provided.

@Alexey19 Alexey19 force-pushed the remove_dependency branch 4 times, most recently from 94628e5 to fb13edf Compare March 9, 2021 06:10
@fetinin
Copy link
Contributor

fetinin commented Mar 11, 2021

Сейчас команда go mod tidy вернет эти зависимости, надо этот чекер именно выносить в отдельный го модуль.

Вот пример проекта который такой подход использует: https://github.com/cristalhq/aconfig
Там у него есть файл декодеры и их нужно отдельно ставить. Так как раз сделано чтобы ненужные зависимости не тащить.
Например, если тебе надо работать с .env файлами, то ты делаешь go get https://github.com/cristalhq/aconfig/aconfigdotenv и потом импортируешь этот декодер у себя в проекте.
Нам здесь по аналогии нужно сделать.

@fetinin fetinin added enhancement New feature or request minor release as minor labels Mar 11, 2021
@Alexey19 Alexey19 force-pushed the remove_dependency branch 2 times, most recently from 92520a1 to f666bbb Compare March 12, 2021 04:17
@Alexey19
Copy link
Contributor Author

Alexey19 commented Mar 12, 2021 •
edited

Да, сработало, перенес в отдельный модуль, заодно добавил модуль для openapi3. Пришлось правда переносить все в другой каталог, т.к. checker содержит интерфейс.

@fetinin
Copy link
Contributor

fetinin commented Mar 12, 2021

Отлично! :)

Только с папкой checker_addon один момент, мне кажется предпочтительнее не создавать лишний раз новых папок в корне проекта и убрать отдельно устанавлемые чекеры в checker/addons/.

И, если надутся силы, добавь пожалуйста тесты. 🙏 Хотя бы на новый функционал с валидацией openapi3 который ты привнес.

@Alexey19
Copy link
Contributor Author

Перенес в checker/addons. Свой новый чекер я пока удалил: погоняю у нас, пойму как у него конструктор будет выглядеть и чуть позже в виде отдельного пул-реквеста добавлю.

fetinin
fetinin approved these changes Mar 18, 2021
View reviewed changes
Copy link
Contributor

@fetinin fetinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fetinin fetinin merged commit da030a1 into lamoda:master Mar 19, 2021
@github-actions
Copy link

🚀 PR was released in v1.4.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fetinin fetinin fetinin approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request minor release as minor released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Alexey19 @fetinin