Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Methods pointing to the "uncustomizable" classes. #2861

Merged
merged 7 commits into from
Nov 3, 2022

Conversation

OzanKurt
Copy link
Contributor

@OzanKurt OzanKurt commented Oct 3, 2022

This allows people to use corresponding method calls like:

$builder = datatables()->eloquent($query);

instead of needing to use the make method.

$builder = datatables()->make($query);

Since only the make is using the classes from datatables.engines, the code is not consistent.

@yajra
Copy link
Owner

yajra commented Oct 5, 2022

The code looks fine but the pipelines are failing. Can you please fix it? Thanks!

@OzanKurt
Copy link
Contributor Author

Done

@yajra yajra self-requested a review October 15, 2022 04:46
Copy link
Owner

@yajra yajra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpstan is failing since the class is now being fetched from config. There is a chance that the package will fail if the config is wrong.

<file name="src/DataTables.php">
  <error line="117" column="1" severity="error" message="Cannot call static method create() on mixed." />
  <error line="130" column="1" severity="error" message="Cannot call static method create() on mixed." />
  <error line="[14](https://github.com/yajra/laravel-datatables/actions/runs/3254410336/jobs/5342627471#step:5:15)3" column="1" severity="error" message="Cannot call static method create() on mixed." />
</file>

Maybe we can ignore this for it to pass? https://phpstan.org/user-guide/ignoring-errors#ignoring-in-code-using-phpdocs

/** @phpstan-ignore-next-line */

...so that people don't get confused in case they forget to extend the base datatable engines.
@OzanKurt
Copy link
Contributor Author

OzanKurt commented Nov 1, 2022

I'm sorry mate, I can't deal with these nonsense phpstan errors. I even tried installing the package standalone and it's just too much waste of time.

image

@sonarcloud
Copy link

sonarcloud bot commented Nov 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
16.2% 16.2% Duplication

@yajra
Copy link
Owner

yajra commented Nov 2, 2022

Looks good and thanks for the hassle. Will review this as soon as I can.

@yajra yajra merged commit 94ca929 into yajra:master Nov 3, 2022
@yajra
Copy link
Owner

yajra commented Nov 3, 2022

Released on v10.2.0, thanks a lot! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants