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.

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

Hw13 calendar #13

Merged
merged 27 commits into from
Oct 6, 2021
Merged

Hw13 calendar #13

merged 27 commits into from
Oct 6, 2021

Conversation

spendmail
Copy link
Owner

No description provided.

Copy link
Collaborator

@kulti kulti left a comment

Choose a reason for hiding this comment

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

Константин, спасибо за выполнение ДЗ.

Ничего критичного не нашел. Можно двигаться дальше в работе над календарем. :)

hw12_13_14_15_calendar/Makefile Show resolved Hide resolved
hw12_13_14_15_calendar/api/EventService.proto Outdated Show resolved Hide resolved
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

// Storage initialization.
storage, err := factorystorage.GetStorage(ctx, config)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Сторадж вроде делают в предыдущем ДЗ? Я так понял, тут или какой-то рефакторинг или случайно коммиты попали. В ревью сосредоточился на части, связанной с API. Если в сторадже тоже есть что-то важное, что вы хотите показать, напишите, я посмотрю.

hw12_13_14_15_calendar/cmd/calendar/main.go Outdated Show resolved Hide resolved
hw12_13_14_15_calendar/cmd/calendar/main.go Outdated Show resolved Hide resolved
// TODO
int64 id = 1;
string title = 2;
string begin_date = 3;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Как вариант можно использовать google.protobuf.Timestamp.

case <-signals:
// Locking until OS signal is sent or context cancel func is called.
<-ctx.Done()
cancel()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Если контекст уже Done, то нет смысла еще раз делать cancel() :)

@spendmail spendmail merged commit 0d1fe5b into master Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants