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 PSR-4 #3

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Add PSR-4 #3

wants to merge 6 commits into from

Conversation

envomer
Copy link
Contributor

@envomer envomer commented Aug 12, 2021

This PR adds PSR-4 support to the src path and also adds GitHub action that triggers the kahlan testing pipeline.

PS: the tests are still failing because of a namespacing issue in regards to Eno\\Section. I'll continue investigating but for now i'll create a draft PR.

I also think maybe it's better to create a dedicated PR for the GitHub pipeline so we keep the PRs clean. Sorry about that.

@simonrepp
Copy link
Member

Thanks so much for all the effort! I went over everything and I'll leave two small remarks, but even so during reviewing I just realized that most of the PHP I've picked up while writing enophp (and the incomplete php port of enolib) has already made way to other things in my head again, unfortunately. So consequently I'll pretty much follow your judgement to make changes here - you're the expert and I appreciate your involvement and support. 👍

That said:

  • I assume the change to the autoload path and the thereby increased verbosity of include paths (e.g. Eno\Section -> Eno\Elements\Section) has technical reasons (e.g. unbreaking tests or such) and is not done for its own sake? If so I'd be curious if there'd be any way in PHP to have both? I.e. is it possible to have the orignal, nicely short include paths without whichever problematic sideeffects I probably introduced by the way I implemented it? If on the other hand you just wanted the namespacing to be more verbose, I don't really have a strong opinion there, ok with me too. :)
  • I might be moving various of the eno-lang repos away from github in the future (likely to codeberg.org), for that reason specifically and also for general political/ethical reasons I (at least personally for my work across repos) won't introduce any further dependencies that create a vendor lock-in to github. If you want to add github actions here I'm ok with that, but I'll say also that I'm not going to invest any time into them, and if anything about them breaks I'll just mention you on the issue and delegate any work related to that setup. ;) (I do appreciate the push for professionality with this though, just to say that clearly!)

That's it from my side for now, thanks again for this PR draft!

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

Successfully merging this pull request may close these issues.

2 participants