Adjust project name validations #247
Conversation
xla
force-pushed
the
xla/239-adjust-project-name-validation
branch
from
d7da009
to
693b7c9
Compare
|
Btw, @xla, this is a good example of a PR where I'd rather keep some history instead of squashing everything. Please keep at least one commit for the changes around |
xla
force-pushed
the
xla/239-adjust-project-name-validation
branch
from
5ee4f52
to
1035f98
Compare
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| Self::from_string(s.to_string()) | ||
| } | ||
| } |
There was a problem hiding this comment.
It's too bad we need both FromStr and TryFrom<&str>
There was a problem hiding this comment.
I'm honestly curious, why?
There was a problem hiding this comment.
They provide different APIS (from_str and try_from("") respectively) I saw a FromStr implementation. It seems the main reason for FromStr being around and distinct from TryFrom<T> is for parse semantics: rust-lang/rfcs#2143 (comment)
I'm not certain about our requirements, but happy to settle on of the methods.
There was a problem hiding this comment.
I’d prefer to have one way to do it with TryFrom but I’m ok if we leave it. If we change it here we should align it with the org ID.
There was a problem hiding this comment.
Did an audit of the codebase for occurrences of from_str and found none. Lo and behold, structopt derivation needs it anyway. I suspect for the parse semantics mentioned above. Will leave it for now and we can revise later.
| pub fn what(&self) -> &'static str { | ||
| "" | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we use thiserror here?
There was a problem hiding this comment.
I still need input what we need want and what is possible in the no_std context.
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| Self::from_string(s.to_string()) | ||
| } | ||
| } |
There was a problem hiding this comment.
I’d prefer to have one way to do it with TryFrom but I’m ok if we leave it. If we change it here we should align it with the org ID.
Second part of the name validation changes in radicle-dev/registry-spec#10. Also incorporates the recent addition from radicle-dev/registry-spec#17. Implementation has been mad symmetrical to OrgId Closes #239
xla
force-pushed
the
xla/239-adjust-project-name-validation
branch
from
f5afb76
to
35957cf
Compare
Closes #239 - second part addressing project names.