-
Notifications
You must be signed in to change notification settings - Fork 2
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
フォームドメインのリファクタリング #634
フォームドメインのリファクタリング #634
Conversation
fb0588c
to
5cb7ed4
Compare
…WithContext に包んで返す
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.
Copilot reviewed 90 out of 105 changed files in this pull request and generated 1 comment.
Files not reviewed (15)
- server/domain/src/form/message.rs: Evaluated as low risk
- server/domain/src/form/answer/settings.rs: Evaluated as low risk
- server/domain/src/form.rs: Evaluated as low risk
- server/domain/src/form/answer/models.rs: Evaluated as low risk
- server/domain/src/repository/form/answer_label_repository.rs: Evaluated as low risk
- server/domain/src/repository/form.rs: Evaluated as low risk
- server/domain/src/form/comment/models.rs: Evaluated as low risk
- server/domain/src/repository.rs: Evaluated as low risk
- server/domain/src/notification/models.rs: Evaluated as low risk
- server/domain/Cargo.toml: Evaluated as low risk
- server/domain/src/form/question.rs: Evaluated as low risk
- server/domain/src/form/comment.rs: Evaluated as low risk
- server/Makefile.toml: Evaluated as low risk
- server/domain/src/form/answer.rs: Evaluated as low risk
- server/domain/src/form/question/models.rs: Evaluated as low risk
Comments suppressed due to low confidence (4)
server/domain/src/form/service.rs:33
- [nitpick] The function name 'embedded_answer_title' is ambiguous. It should be renamed to 'generate_embedded_answer_title' for better clarity.
fn embedded_answer_title(
server/domain/src/form/comment/service.rs:39
- The logical combination of AND (&&) and OR (||) operators might lead to unintended behavior. Consider adding parentheses to make the logic explicit:
&& (self.commented_by().id == actor.id || actor.role == Administrator)
.
&& self.commented_by().id == actor.id || actor.role == Administrator
server/domain/src/form/comment/service.rs:49
- The logical combination of AND (&&) and OR (||) operators might lead to unintended behavior. Consider adding parentheses to make the logic explicit:
&& (self.commented_by().id == actor.id || actor.role == Administrator)
.
&& self.commented_by().id == actor.id || actor.role == Administrator
server/domain/src/form/answer/settings/models.rs:15
- The
DerivingVia
attribute is incorrectly formatted. It should be#[deriving(Clone, From, Into, IntoInner, Serialize, Deserialize)]
.
#[deriving(From, Into, IntoInner, Serialize(via: Option::<NonEmptyString>), Deserialize(via: Option::<NonEmptyString>))]
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.
Copilot reviewed 105 out of 105 changed files in this pull request and generated no comments.
一旦この PR の目的だったフォームドメインの整理は達成できたのでマージします |
フォームに付随する機能すべてがフォームドメインとしてまとめられていて、これによって不都合が出てきたのでリファクタリングします