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

Add support for ignoring columns in the database #62

Merged
merged 8 commits into from
Jun 5, 2024

Conversation

grantholle
Copy link
Contributor

This adds support to ignore some columns when saving the page view record, i.e. ignoring the host column since I have a single tenant app and don't need it to bloat the table.

I also changed the test function names to get rid of the deprecation warning about metadata.

@andreaselia
Copy link
Owner

Hey @grantholle, thanks for your PR. I'll try and take a proper look and test things out over the weekend.

Would you mind reverting the test names to keep the /** @test */ way of doing things rather than test_ please? Alternatively we could roll with the #[Test] attribute since I believe from the Laravel 11 requirements we can do that.

@grantholle
Copy link
Contributor Author

I would going to use the #[Test] attribute, but then the package would have to drop support for php 7.4, as it's an 8.0 feature.

@andreaselia
Copy link
Owner

andreaselia commented Apr 18, 2024

We can drop support for PHP 7.4, that works for me and I imagine the majority of users of the package.

@grantholle
Copy link
Contributor Author

Yeah I agree. Since v2.0.0 is already tagged and is php ^8.0, might as well just go with that major version. I think you can still support the older versions of Laravel, but just bump the php requirement.

@grantholle
Copy link
Contributor Author

Ok @andreaselia, should be all set now

@andreaselia
Copy link
Owner

Thanks @grantholle, apologies about the delay.

@andreaselia andreaselia merged commit bef7133 into andreaselia:main Jun 5, 2024
2 checks passed
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.

3 participants