Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

Implements member and members repository #13

Merged
merged 1 commit into from
Jan 31, 2019
Merged

Implements member and members repository #13

merged 1 commit into from
Jan 31, 2019

Conversation

Kirguir
Copy link
Contributor

@Kirguir Kirguir commented Jan 17, 2019

Add struct for member and members repository implemented on actix actor.
Repository handle GetMember and GetMemberByCredentials messages for retrieve member by its id or credentials.

@Kirguir Kirguir self-assigned this Jan 17, 2019
@Kirguir Kirguir added feature New feature or request k::design Related to overall design and/or architecture labels Jan 17, 2019
@Kirguir Kirguir added this to the 0.1.0 milestone Jan 17, 2019
@Kirguir Kirguir requested a review from alexlapa January 17, 2019 12:07
@alexlapa alexlapa requested a review from tyranron January 18, 2019 05:43
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

@Kirguir merge with master after !12 and resend for a review.

Copy link
Contributor Author

@Kirguir Kirguir left a comment

Choose a reason for hiding this comment

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

@tyranron merged with !12

@alexlapa alexlapa mentioned this pull request Jan 18, 2019
16 tasks
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Consider feedback below.

src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
@Kirguir
Copy link
Contributor Author

Kirguir commented Jan 25, 2019

@tyranron изменения запушил, как повторно отправить на ревью?

Copy link
Contributor Author

@Kirguir Kirguir left a comment

Choose a reason for hiding this comment

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

@alexlapa alexlapa requested a review from tyranron January 25, 2019 15:08
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

@Kirguir please, more attention to code style. All missed docs should be filled up.

@alexlapa there is something strange with the idea of baking MemberRepository into Actor. You should carefully reconsider it.

src/main.rs Show resolved Hide resolved
src/errors/mod.rs Outdated Show resolved Hide resolved
src/errors/mod.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Kirguir Kirguir left a comment

Choose a reason for hiding this comment

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

@tyranron removed implementation of actor for member repository. Implemented simple functions for gets member from repo.

Copy link
Contributor Author

@Kirguir Kirguir left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

Still, there were a lot of missing docs. I've fixed this up.

However, implemented API requires to be refactored and simplified.

src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
src/api/control/member.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Kirguir Kirguir left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

With changes now it's OK.

However, I don't see any FCM.

src/api/control/member.rs Outdated Show resolved Hide resolved
@Kirguir
Copy link
Contributor Author

Kirguir commented Jan 31, 2019

@tyranron FCM

Implement member entity and member repository (#13, #10)

- add default caller and responder into repository

- add default caller and responder into repository
@Kirguir Kirguir changed the title WIP: Implements member and members repository Implements member and members repository Jan 31, 2019
@Kirguir Kirguir merged commit faca6d3 into master Jan 31, 2019
@Kirguir Kirguir deleted the add_member branch January 31, 2019 10:27
@tyranron
Copy link
Member

@Kirguir do not use force-push on GitHub. It erases the whole PR history. GitHub has "squash merge" button. Use it directly. This has been mentioned in workflow, please re-read.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature or request k::design Related to overall design and/or architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants