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

Extract interface for route configuration #276

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

lcobucci
Copy link
Collaborator

@lcobucci lcobucci commented Feb 6, 2024

Fixes #209

This introduces an extension point for providing different implementations for the configuration of application routes.

Lock file operations: 0 installs, 10 updates, 0 removals
  - Upgrading doctrine/lexer (3.0.0 => 3.0.1)
  - Upgrading phpunit/phpunit (10.5.9 => 10.5.10)
  - Upgrading symfony/console (v6.4.2 => v6.4.3)
  - Upgrading symfony/filesystem (v6.4.0 => v6.4.3)
  - Upgrading symfony/polyfill-ctype (v1.28.0 => v1.29.0)
  - Upgrading symfony/polyfill-intl-grapheme (v1.28.0 => v1.29.0)
  - Upgrading symfony/polyfill-intl-normalizer (v1.28.0 => v1.29.0)
  - Upgrading symfony/polyfill-mbstring (v1.28.0 => v1.29.0)
  - Upgrading symfony/process (v6.4.2 => v6.4.3)
  - Upgrading symfony/string (v6.4.2 => v6.4.3)

Signed-off-by: Luís Cobucci <[email protected]>
This introduces an extension point for providing different
implementations for the configuration of application routes.

Signed-off-by: Luís Cobucci <[email protected]>
@lcobucci lcobucci added this to the 2.0.0 milestone Feb 6, 2024
@lcobucci lcobucci self-assigned this Feb 6, 2024
@lcobucci lcobucci merged commit 0e4448b into master Feb 6, 2024
33 checks passed
@lcobucci lcobucci deleted the extract-interface-for-route-configuration branch February 6, 2024 21:23
@jesperbeisner
Copy link

Hey @lcobucci, I tried to copy the init setup from the README after my composer installation, but this doesn't work anymore, cause the ConfigureRoutes is not available yet. Are you aware of that? I mean I know how to fix this - aka. use the old RouteCollector - but new users may not know this and will run into problems, cause the README is not up2date with the composer installation? 😿

Nonetheless, thanks for your work on FastRoute 💪

@lcobucci
Copy link
Collaborator Author

Hey @lcobucci, I tried to copy the init setup from the README after my composer installation, but this doesn't work anymore, cause the ConfigureRoutes is not available yet. Are you aware of that? I mean I know how to fix this - aka. use the old RouteCollector - but new users may not know this and will run into problems, cause the README is not up2date with the composer installation? 😿

Nonetheless, thanks for your work on FastRoute 💪

@jesperbeisner this hasn't yet been released 😅

I got sick and couldn't continue my work on #277, which is the last thing before I wrap up an RC tag.

You may point to 2.0-dev@dev if you want to test this out. Alternatively, refer to the docs on the latest tag 👍

@jesperbeisner
Copy link

jesperbeisner commented Feb 20, 2024

I know this hasn't been released yet, that's exactly my point 😄
I visit the github repository, I run composer require nikic/fast-route, I try the example, nothing works cause the default master branch on github !== my locally installed version, I get sad 😿

Was just wondering if this is intentional? And yes, I know how to "fix" this but maybe people new to php/github/composer do not and will get annoyed?

Maybe the 2.0 development should happen in the 2.0 branch and master is the current 1.X version?

@lcobucci
Copy link
Collaborator Author

I know this hasn't been released yet, that's exactly my point 😄
I visit the github repository, I run composer require nikic/fast-route, I try the example, nothing works cause the default master branch on github !== my locally installed version, I get sad 😿

Was just wondering if this is intentional? And yes, I know how to "fix" this but maybe people new to php/github/composer do not and will get annoyed?

Maybe the 2.0 development should happen in the 2.0 branch and master is the current 1.X version?

Generally speaking, I've moved away from "master" or "main" branches in my libs, using branches like "1.0.x" and having the version under development as the default one.

Unfortunately only the repository owner can change the default branch, so it would be quite messy to have to ask Nikita to change stuff all the time 😅

The outcome would be the same, though: since the default branch is the version under development, documentation won't necessarily match with what you have installed locally.

So, one can say this is by design and mitigated by having docs in the dist file (so people can consume the documentation locally, from their vendor dir).

The main argument for that is reduce contribution friction - we want people to push things to the version under development as much as possible.

I hope that makes sense 🤘

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

Successfully merging this pull request may close these issues.

Need for a RouteCollectorInterface
2 participants