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-08-06 14:01:58 +03:00
WIP: feature_paste

Why double quotes again?

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

Same here.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

It's better not to rely on external CDN's when you can.

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

Again why HTML here?

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 commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

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

skobkin commented on pull request Miroslavsckaya/copypaste#1 2023-08-06 14:01:58 +03:00
WIP: feature_paste

Why do you use HTML here and not in the template? Check the comment I left in PasterController::showPaste().

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 not forget to use code formatting. It'd surrounded = with spaces for you.

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 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 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

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

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

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

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

Huh?

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

Or you can make constructor private, use property promotion and add fromPaste() method to create it from the entity.

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 suggested changes for Miroslavsckaya/copypaste#1 2023-07-22 01:41:59 +03:00
WIP: feature_paste

Good job. A couple more things.