-
Notifications
You must be signed in to change notification settings - Fork 1
Cs 16 #25
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
base: master
Are you sure you want to change the base?
Cs 16 #25
Conversation
Dockerfile
Outdated
| @@ -0,0 +1,36 @@ | |||
| FROM golang:1.19.3 as app_builder | |||
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.
alpine?
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.
Debian bullseye -
ок на alpine переделаю
api/proto/certificate.proto
Outdated
| Status status = 1; | ||
| } | ||
|
|
||
| message DelTemplateResp { |
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.
Не стоит скоращатьназвания - потом больно будет.
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.
исправил
| package server | ||
|
|
||
| import ( | ||
| "context" |
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.
minor: мы обычно сортируем импорты иначе
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.
Вручную сортируете?
У меня сами gofmt и goimports сортируют.
Как нужно?
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.
исправил
| return server, nil | ||
| } | ||
|
|
||
| func (c *certificateServer) IssueCertificate(ctx context.Context, req *certSPb.IssueCertificateReq) (*certSPb.IssueCertificateResp, error) { |
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.
Разбить эту функцию. Очень уж длиннная
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.
часть функционала вытащил в метод fillData. Еще разбить?
app/server/server.go
Outdated
| } | ||
|
|
||
| // Генерация ID/имени сертификата (ID + ".pdf"). | ||
| nameCertificateIdPDF := c.certGen.GenerateID() + ".pdf" |
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.
Тут бы по-хорошему usecase вставить, так получается сильная связность между сервером(транспорт) и бизнес логикой.
Пока можно и так, но это уэе хороший запрос на сказ о чистой архитектуре.
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.
Изменил, сделал так:
certificateId := c.certGen.GenerateID()
Расширение файла добавляется только при отправке и поиске в хранилище:
certificateId+certificateFileExtension
cmd/rpc/main.go
Outdated
| ) | ||
|
|
||
| func main() { | ||
| host := "0.0.0.0" |
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.
в env!
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.
сделал
|
|
||
| import "github.com/skip2/go-qrcode" | ||
|
|
||
| const sizeImage = 128 |
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.
по идее это свойство шаблона
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.
Да, но при генерации QR нужно указать какой-то размер. Поставил 128 как оптимальный.
В шаблоне же указываются размер <img src={{.QrCodeLink}} width="128" height="128">
И наш QR автоматически впишется в эти размеры.
Чтоб генерировать QR размером как в шаблоне можно конечно из HTML шаблона вытащить его размеры.
Но создатели шаблона могут вообще размеры не задавать, тогда QR будет в документе того размера как есть.
Сделать так, брать размеры QR из шаблона?
CS-16
What:
Реализована работа сервиса в Docker контейнере.
Why:
Для быстрого развертывания в любой инфраструктуре.
How:
В корень проекта добавлены файлы: Dockerfile, .dockerignore, protoc_options_generate.txt
В файл README.md добавлена инструкция запуска сервиса в Docker контейнере.