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

Getting rid of a circular dependency between bijection-macros and chill #228

Merged
merged 2 commits into from
Sep 3, 2015

Conversation

sriramkrishnan
Copy link
Collaborator

Bijection and Chill now have a circular dependency between each other. This is because com.twitter.chill.Externalizer is used for the bijection-macros test.

I cherrypicked and simplified the Externalizer, and moved it into the TestHelpers in bijection-macros.

@johnynek
Copy link
Collaborator

johnynek commented Sep 3, 2015

Actually, I'm not so sure this is the best approach.

I think bijection-macros should be killed. We should move the macros into bijection (no need for a new package). Also, the IsCaseClass and MacroGenerated traits are really not useful. They were something generated when we were making the first few macros, but they are not valuable, as far as I can see (we later noticed other ways to do the same thing).

@johnynek
Copy link
Collaborator

johnynek commented Sep 3, 2015

That said, we could merge this and just take on killing bijection-macros as an issue.

@sriramkrishnan
Copy link
Collaborator Author

@johnynek yeah - my preference would be to kill bijection-macros as a separate issue. Thanks for looking at this.

@johnynek
Copy link
Collaborator

johnynek commented Sep 3, 2015

added #229

johnynek added a commit that referenced this pull request Sep 3, 2015
Getting rid of a circular dependency between bijection-macros and chill
@johnynek johnynek merged commit ef6b049 into twitter:develop Sep 3, 2015
@sriramkrishnan
Copy link
Collaborator Author

That was quick - thanks!

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