iam-omer-mahdi / ss-v10

0 stars 0 forks source link

General things #5

Open ModestasV opened 1 year ago

ModestasV commented 1 year ago

Some things don't really deserve their own issue as they are more or less opinionated so I'll group them together :) Or just general feedback/tips for you!

Using request classes I've noticed that you use $request->field_name instead of $request->input('field_name') and while I understand that it's easier and works - be careful as they might break if you touch a reserved field name :)

Not using code formatters While looking at the code I've noticed that you have quite a bit of inconsistency in your code styling. There are some spaces, tabs all over the place that don't really match one style. While it's not a bug/issue directly - it can lead to issues. For example, did you know that you have a space here? https://github.com/iam-omer-mahdi/ss-v10/blob/main/app/Http/Controllers/StudentPartController.php#L132

 $file-> move(public_path('images/payment'), $filename);

I'd personally recommend you attempt to install https://github.com/laravel/pint with the default Laravel preset and run it on each commit :)


Overall the code is ok, I don't think that there's too much to go after initially. It doesn't seem to be too complex and contains all the things nicely :)

iam-omer-mahdi commented 1 year ago

Thank you very much for your feedback i really appreciate it i will try to improve the things you've mentioned ☺️

On Tue, Mar 21, 2023, 9:13 AM Modestas Vaitkevičius < @.***> wrote:

Some things don't really deserve their own issue as they are more or less opinionated so I'll group them together :) Or just general feedback/tips for you!

Using request classes I've noticed that you use $request->field_name instead of $request->input('field_name') and while I understand that it's easier and works - be careful as they might break if you touch a reserved field name :)

Not using code formatters While looking at the code I've noticed that you have quite a bit of inconsistency in your code styling. There are some spaces, tabs all over the place that don't really match one style. While it's not a bug/issue directly - it can lead to issues. For example, did you know that you have a space here?

https://github.com/iam-omer-mahdi/ss-v10/blob/main/app/Http/Controllers/StudentPartController.php#L132

$file-> move(public_path('images/payment'), $filename);

I'd personally recommend you attempt to install https://github.com/laravel/pint with the default Laravel preset and run it on each commit :)

Overall the code is ok, I don't think that there's too much to go after initially. It doesn't seem to be too complex and contains all the things nicely :)

— Reply to this email directly, view it on GitHub https://github.com/iam-omer-mahdi/ss-v10/issues/5, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALRNM5VD7TR7LARLJU37KCLW5FIK7ANCNFSM6AAAAAAWCBFO64 . You are receiving this because you are subscribed to this thread.Message ID: @.***>