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

Embed Hoa\Compiler #1184

Closed
wants to merge 5 commits into from
Closed

Conversation

sanmai
Copy link
Contributor

@sanmai sanmai commented Apr 6, 2020

  • TODO: description is outdated

I've been using this fork for some time in production, and necessary to say I took at least some effort to make sure it works as it should. For example, tests were migrated to PHPUnit, but we still run the original test suite to make sure it's even.

There's a slight breaking change for packages that depend both on the serializer and the compiler: Hoa\Exception\Exception-type exceptions are no longer thrown as the compiler does not depend on the exception abstractions it previously used. This is due to I tried to minimize the number of potentially abandoned dependencies, removing unnecessary for day to day operations.

Second breaking change is in the fact that the compiler package no longer includes dev dependecies as non-dev. E.g. if one wants to use getAST() to make new Ln compilers on the fly, they'll need to include additional dependencies. The serializer is not affected.

The above change also makes the serializer non-dependent on hoa/consistency, therefore hoaproject/Consistency#35 is no longer a problem when using the serializer.

Currently I'm the sole maintainer for the fork, but I'll be happy to accept a co-maintainer or two. (CI procedure isn't as robust as I'd want it, but that's not a big deal to fix. We'll fix it once we get there.)

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? yes TBD
Deprecations? no
Tests pass? yes
Fixed tickets #1182
License MIT

Fixes #1182

sanmai added 2 commits April 13, 2020 04:05
To update namespace use:

find src/Type/ -type f | xargs sed -i 's:Hoa\\Compiler:JMS\\Serializer\\Type\\Compiler:g'
@goetas
Copy link
Collaborator

goetas commented Apr 12, 2020

Personally I would have preferred if the fork was using explicitly a different namespace... so it looked just as a different package.

@sanmai
Copy link
Contributor Author

sanmai commented Apr 13, 2020

Fair enough. Currently it uses original Hoa\Compiler namespace. What do you think would work better?

@sanmai
Copy link
Contributor Author

sanmai commented Apr 13, 2020

Never mind previous question, we can include the bulk of the compiler package in a sub-namespace of the very package. Updated dependencies are only hoa/iterator and hoa/visitor.

@sanmai sanmai changed the title Use forked version of Hoa\Compiler Embed Hoa\Compiler Apr 13, 2020
@goetas goetas closed this in #1212 Jun 13, 2020
@sanmai sanmai deleted the pr/2020-04/hoa-compiler branch June 13, 2020 09:56
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.

[RFC] Removing abandoned hoa from serializer
2 participants