Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

chore: install paper_elements & core_elements from pub #1479

Closed
wants to merge 2 commits into from

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Sep 19, 2014

fixes #1281
fixes #1283

@vicb
Copy link
Contributor Author

vicb commented Sep 19, 2014

There is an issue as the expression extractor will extract expression from paper elements now (polymer also uses mustache).
Looking into this

@mhevery mhevery added cla: yes and removed cla: no labels Sep 19, 2014
@vicb
Copy link
Contributor Author

vicb commented Sep 19, 2014

@jbdeboer I've added a second commit to fix the generator. I'm curious to know if this will work with g3. If not we could be more specific on the folder we want to blacklist. This commit could could help making the generator step faster.

@naomiblack naomiblack added this to the v1.0 milestone Oct 2, 2014
@naomiblack
Copy link
Contributor

@jbdeboer can you review and approve/comment, or else defer to post-1.0?

@jbdeboer
Copy link
Contributor

jbdeboer commented Oct 3, 2014

Angular itself does not depend on paper_elements, so it should not be in the main pubspec.

The question is whether it should go in the examples pubspec. I would say no. The demo shows AngularDart and PolymerJS working together -- having PolymerJS pulled from bower (the recommended PolymerJS distribution path) clearly demonstrates that you do not need Dart wrappers to use Javascript web components.

That said, this is something that you feel strongly about. Perhaps we can come up with additional or different demos and clarify the intent.

@vicb
Copy link
Contributor Author

vicb commented Oct 3, 2014

@jbdeboer The deps should be in the examples pubspec, this is a mistake on my side.

I think this change (once the deps are moved at the right) is good because:

  • it forces us to have the angular pubspec compatible with the polymer packages,
  • we don't have to install yet an other pkg manager (bower),
  • users can choose their preferred way for installing polymer (either bower or pub).

Without this change user won't be able to use pub (because of version conflicts) and will be forced to use bower.

I don't get the part about adding more demos.

I think this change could make our users happier. Let me know if I need to update this PR (wrt example pubspec) and any other thing I need to update or if this is not worth it.

@naomiblack
Copy link
Contributor

@jbdeboer this needs a decision to merge or should be closed. Can you make the final call please?

@jbdeboer
Copy link
Contributor

jbdeboer commented Oct 7, 2014

Your first point of forcing compatibility is excellent. We should do that, but we should do it in another example project, so that we have demos of:

  • AngularDart <-> PolymerJS
  • AngularDart <-> PolymerDart

I am closing this PR. @vicb, if you would like to add a new AngularDart <-> PolymerDart example, please open a new PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

Consider updating html5lib version dependency Use the paper_element pub package to install deps
4 participants