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.

Sign up for GitHub

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

Jump to bottom

Make reagent self-host compatible #283

Merged
merged 3 commits into from
May 19, 2017
Merged

Make reagent self-host compatible #283

merged 3 commits into from
May 19, 2017

Conversation

viebel
Copy link
Contributor

@viebel viebel commented Dec 31, 2016

This PR contains 2 changes that makes reagent self-host compatible:

  1. remove JVM related code in macro files
  2. remove deprecated macro code as it causes trouble in self-host cljs. See the problematice code in klipse

danielcompton
danielcompton approved these changes Jan 2, 2017
View reviewed changes
Copy link
Contributor

@danielcompton danielcompton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fairly innocuous, will need to check how long the macros have been deprecated for. I'm on mobile, so can't really tell.

src/reagent/interop.clj
@@ -56,7 +55,7 @@
(assert field (str "Field name must start with - in " field))
`(aset ~object ~@names ~value)))

(defmacro .' [& args]
#_(defmacro .' [& args]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to comment them out, we should probably just remove them?

@viebel
Copy link
Contributor Author

viebel commented Jan 2, 2017

@danielcompton I have removed the deprecated code.

@pesterhazy
Copy link
Contributor

I just ran into the same issue when trying to get Reagent to work from lumo

@pesterhazy pesterhazy mentioned this pull request Jan 7, 2017
Closed
@danielcompton
Copy link
Contributor

The previous macros were deprecated in 0.6.0, so I'm not sure how much time we want to give.

@Deraen Deraen added this to the 0.7.0 milestone Mar 10, 2017
@viebel
Copy link
Contributor Author

viebel commented Mar 11, 2017

would you like to merge this PR?

@viebel viebel mentioned this pull request Mar 11, 2017
Merged
@Deraen
Copy link
Member

Deraen commented Mar 12, 2017

@viebel I'll merge this when I start working on a new version.

@Deraen Deraen merged commit bc67773 into reagent-project:master May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielcompton danielcompton danielcompton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@viebel @pesterhazy @danielcompton @Deraen