Alexey Skobkin skobkin
0 followers · 4 following
Repositories
22
 Projects Packages Public activity Starred repositories
4
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

Do you need it here if it's empty?

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

Why not nullable?

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

required => false?

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 00:56:34 +03:00
WIP: feature_paste

Better with optional flush. Flushng from the repo itself should be a rare action.

skobkin suggested changes for Miroslavsckaya/copypaste#1 2023-07-20 21:41:13 +03:00
WIP: feature_paste

I left a couple of suggestions for improvement.

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

Either describe what these migrations do or remove meaningless comments.

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

I'd suggest adding save() here like I showed you.

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

Can't leave a comment for src/Repository/.gitignore, so I'll ask here.

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

Please do not forget declare(strict_types=1);

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

This should be a combo-box (select in HTML terms) with a list of available periods like "10 minutes", "1 year" and "Never".

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

You can probably extract 128 to the constant and use here and in the validation attribute.

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

Double quotes?

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

Please do not forget declare(strict_types=1);

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

Do you need double quotes here too?

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

You can just set it in the constructor.

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

I'd suggest we refactor this to to DTO usage and make our Paste entity immutable.