cargo new does most of the scaffolding for us. i made a small change to the .gitignore file to make sure we don't check in targets. we do, however, want to check in the Cargo.lock file since this is a binary.
Details
Details
cargo build && cargo run works.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I understand this will work the same as other services, right? So we're going to build this inside of a docker container? Or are you planning to enable building on host macs as well? Or only on host macs? Could you clarify what's the plan here? Thanks.
Back to your queue.
Comment Actions
yeah, i plan to containerize this application. this is coming later in my diff stack. ideally a developer should be able to run the binary without a container as well with cargo run.
Comment Actions
yeah, i plan to containerize this application. this is coming later in my diff stack. ideally a developer should be able to run the binary without a container as well with cargo run.
If users are supposed to run this on their host machines, maybe it is worth adding some doc on how they should set up a build env?
Comment Actions
I don't think we should make a "Hello world" diffs or at least mark it as a draft.
I think we should make a diff when it comes to some functionality or a full project skeleton at least and omit such hello worlds.
Do we have an architecture for the identity service somewhere?
Comment Actions
What are the changes you're requesting, @karol-bisztyga? I've created a linear task to track updating dev_services.md with instructions for running the Identity service, but that isn't really related to this diff. Re-requesting review
Comment Actions
I didn't find a Linear task for a full architecture review, maybe we should create it to track and comment?
I think the review needs to answer the main questions, before the main implementation:
- gRPC API methods description;
- Architecture about communication and models in third-party's (databases etc.);
- How identity service will communicate with the other services (API);
If we have a clear architecture description first we can avoid re-architecture the service in the future and it's much easier for reviewers to review.
Comment Actions
@varun, I think @karol-bisztyga requested changes just to get an answer to his questions. Not sure you've answered all of them yet (eg. clarifying that yes, this is meant to run on Docker)
| services/identity/src/main.rs | ||
|---|---|---|
| 2 ↗ | (On Diff #10830) | 2-space tabs please! I think you have a diff out for this, but I'm not sure it fixes this file. Might be good to reorder your stack so that the 2-space tabs diff comes first and this file looks correct |
Comment Actions
I thought I did answer them:
yeah, i plan to containerize this application. this is coming later in my diff stack. ideally a developer should be able to run the binary without a container as well with cargo run.
I have a diff out with the Dockerfile for this service: D3593