search

RedDragonWebDesign / BlueThrust5

A fork/update of the famous BlueThrust Clan Scripts v4. Gaming community website engine in PHP.
10 stars 7 forks source link

Fixes for search warnings errors and addressing issue 114 #118

Closed deepend-tildeclub closed 8 months ago

deepend-tildeclub commented 8 months ago

renamed this. that comma was causing issues with my local machines git. Hopefully this doesn't mess everything up.

deepend-tildeclub commented 8 months ago

There now my git should be functioning as intended. Sorry about all the issues. I'm not the best at git.

RedDragonWebDesign commented 8 months ago

Sorry about all the issues. I'm not the best at git.

Don't worry, you're not the only one. I hate git. So unintuitive for beginners. Desperately needs a GUI.

deepend-tildeclub commented 8 months ago

Sorry about all the issues. I'm not the best at git.

Don't worry, you're not the only one. I hate git. So unintuitive for beginners. Desperately needs a GUI.

I'm ok with no GUI but it could at least be a little less convoluted. I run tilde.club which mostly centers around the command line. so I'm generally good with that.

deepend-tildeclub commented 8 months ago

Is this one also too large? Or can this one be manageable.

RedDragonWebDesign commented 8 months ago

Related #114

RedDragonWebDesign commented 8 months ago

Yes, can you split this one too? One patch for the #114 feature, and one patch per page of error fixes. So if you fix all the errors on the search results page, that could be one patch. And if you fix all the errors on the advanced search page, that could be an additional patch.

Adding public $description and public $prefillValues to Form.php looks suspicious since they are never used. And I can't tell what error they're trying to fix. Splitting into more patches that are clear about what errors the patch is fixing would help review this kind of code.

Thank you for being flexible and responsive to my comments. I appreciate it.

deepend-tildeclub commented 8 months ago

Yes, can you split this one too? One patch for the #114 feature, and one patch per page of error fixes. So if you fix all the errors on the search results page, that could be one patch. And if you fix all the errors on the advanced search page, that could be an additional patch.

Adding public $description and public $prefillValues to Form.php looks suspicious since they are never used. And I can't tell what error they're trying to fix. Splitting into more patches that are clear about what errors the patch is fixing would help review this kind of code.

Thank you for being flexible and responsive to my comments. I appreciate it.

I think its possible that only relevant part of this PR is the 114 portion of it.. Since when I made this one I was still having issues with my git that I didn't realize. Now things are good so we push the split the 114 portion of it out and throw the rest away. Hopefully the other PR's I made have a better chance of making it somewhere since I focused on making them as simple and clean as possible. :)