Conversation
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.
Laying down some thoughts to track improvements, changes, and questions :)
We're currently not verifying any of the entities. We should :) |
allows us to pass around a single type and define functions in terms of the capabilities. This meant that I cleared out a lot of passing around of Paths, and instead it passed around the Client. Fixed up errors to use thiserror.
means that we can implement the functionality via the PeerApi rather than have a Client trait. WIP: The tests compile but fail due to some secret key issues.
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.
DOPE!!!!!!!!!!!!!!
Co-authored-by: Alexander Simmerl <a.simmerl@gmail.com>
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.
All good from my perspective 😄
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.
Awesome stuff! 🏖️ 🦺 ✌️
Biggest question is: why do my project disappear after I restart the app?
Inlined a couple more questions.
@@ -15,4 +15,3 @@ RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - | |||
RUN echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list | |||
RUN apt-get -y update | |||
RUN apt-get -y install yarn | |||
RUN rustup component add clippy --toolchain nightly-2020-02-05-x86_64-unknown-linux-gnu |
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.
Does this mean we're not installing the rust toolchain in the image and are instead downloading it every time? If that's true, it will blow up on CI after a while.
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.
Nope, it's just part of the base image.
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.
And does this registry base image have the same rust-toolchain version the coco dependency? Will both dependencies always be on the same version?
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.
We currently don't actively coordinate the toolchain versions across projects. Historically registry was the first to adopt nightly, so we have been trailing theirs. They also off the base image conveniently. This could be raised in radicle-decisions as a cross-engineering concern.
@rudolfs: is this happening running the application in "production" mode? It might be something I need you to walk myself or xla through 👀 |
@FintanH what is the "production mode"? I start it as I've always done with |
That sounds like "production mode" to me 😅 There's a flag that checks are we testing or not and depending on that the setup for coco is different, i.e. uses tmp dirs. I would expect the projects to be persisted so we should look into that. |
Will be addressed in: #456 |
Fixes #434