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

Improve DX using connections. #159

Merged
merged 39 commits into from
Jan 16, 2024
Merged

Improve DX using connections. #159

merged 39 commits into from
Jan 16, 2024

Conversation

vluijkx
Copy link
Contributor

@vluijkx vluijkx commented Jan 8, 2024

Hi again! 😅

We need a setup in which errors are kept in the default sqlite store, while redirects are stored in the main Laravel database. Doing this caused all sorts of DX issues which this PR tries to solve.

Tip

The DOCUMENTATION.md diff explains how this new setup works.

Issues

There is just one connection config for both errors and redirects.

This meant we had to create our own custom repository to use a seperate connection for redirects. This PR adds seperate redirect_connection and error_connection config values.

The default sqlite database is called errors.sqlite

It took a little while until I realized that eloquent redirects are stored here as well. This PR renames the database to redirect.sqlite.

The README.md is outdated

The repository classes mentioned do not exist anymore. This PR removes outdated and duplicate documentation in the README.md and adds updated documentation to the DOCUMENTATION.md.

The connection name redirect was unclear at first

Renaming it to redirect-sqlite clarified this.

The migration name create_redirect_tables implies that it contains all redirect tables

You need to open the migration files to figure out the difference between:

  • create_redirect_tables
  • create_eloquent_redirect_table

This PR renames them to:

  • create_redirect_error_tables
  • create_redirect_redirects_table

The different migrations contain clearer tags. This makes it easier for developers to publish just the migration(s) they need.

Considerations

  • There is backwards compatibility with the connection config. connection currently overrides error_connection and redirect_connection. In my opinion, it would be better to create a new major version and drop connection support.
  • I wanted to create an update script for the errors.sqlite to redirect.sqlite rename. This causes problems as the boot method in the service provider runs before the update scripts. This is the reason why this logic has been put in the boot method.
  • Have not had time yet to write proper tests.

vluijkx and others added 30 commits January 6, 2024 12:24
This reverts commit 397cce2.
People can read the up-to-date documentation instead.
@vluijkx vluijkx changed the title Improve developer DX using connections. Improve DX using connections. Jan 8, 2024
@riasvdv riasvdv merged commit d2f3d61 into riasvdv:main Jan 16, 2024
8 checks passed
@riasvdv
Copy link
Owner

riasvdv commented Jan 16, 2024

Thank you!

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