Alexey Skobkin skobkin
0 followers · 4 following
Repositories
25
 Projects Packages Public activity Starred repositories
4
skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

Why not inject into constructor?

skobkin suggested changes for Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

Good start, but there are things to improve and other things to do.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 13:54:30 +03:00
WIP: feature_paste

Also naming here is not good. You have $highlighted and $highlighted_text.

  • $highlighted_text is not in $camelCase
  • Both variables contain text, both contain code. If it shouldn't be…
skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 21:03:04 +03:00
WIP: feature_paste

Do you need IP in PasteData?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 21:03:04 +03:00
WIP: feature_paste

Is this validation being processed BEFORE storing the data in the DTO?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 21:03:04 +03:00
WIP: feature_paste

Why not also constructor promotion though?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 21:03:04 +03:00
WIP: feature_paste

Do not forget to use code formatting. It'd surrounded = with spaces for you.

skobkin approved Miroslavsckaya/copypaste#1 2023-07-22 21:03:04 +03:00
WIP: feature_paste

Closed old threads, added a couple of comments.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

I mean you don't need to pass creation date here at all.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste
  • it's a good idea to add , to the last argument too if you use multi-line formatting
skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

This still could be done in the entity itself.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

required => false?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

This shouldn't be here.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

IPv6?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

Why not nullable?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

You shouldn't do flush here by default. Only in certain cases if needed.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

I still suggest to make Paste::fromFormData().

skobkin suggested changes for Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

Good job. A couple more things.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

Why not string and Assert\NotBlank?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

Huh?