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

Snap SVG #13

Open
Siyfion opened this issue Mar 10, 2015 · 29 comments
Open

Snap SVG #13

Siyfion opened this issue Mar 10, 2015 · 29 comments

Comments

@Siyfion
Copy link

Siyfion commented Mar 10, 2015

I would like permission to fork this library into the MeteorPackaging org.

@splendido I'm working my way through the wiki now... 😉

@splendido
Copy link
Member

just forked on MeteorPackaging/Snap.svg
and inveted you on the Snap.Svg team.

Hope everything is fine!

@Siyfion
Copy link
Author

Siyfion commented Mar 12, 2015

Sorry @splendido I'm a bit confused as to what I have to do in the forked repo, Snap.svg uses Grunt in it's build system, but I'm not sure what I need to add and where! Probably bitten off a bit more than I can chew, lol.

@splendido
Copy link
Member

@Siyfion you should add a grunt task to update the version number inside the package.js file when they run their main task.

Lets have a look here for an example tast!
Something similar has been done here and called here

Let me know if you need more help!

@splendido
Copy link
Member

@Siyfion, @jshimko, @chip,
lets continue here the discussion at https://github.com/jshimko/meteor-snapsvg/issues/1

@splendido
Copy link
Member

please find the empy repo for the wrapper here
...and see the snapsvg team.

TO avoid sending the PR to the original repo and dig into complicated build systems, we're now trying with the sister repo approach, more or less discussed/described here and here

@splendido
Copy link
Member

Recent attemps by me and @chip are jspdf-core-wrapper and jspdf-AutoTable-wrapper.

The idea is to have a Gulpfile to easily retrieve a particular release of the upstream repo and consequently update the package.js file to reflect the correct version number.

The file autopublish.json is being intoduced to aid the Gulp tasks and hence easy the publish process for autopublish.js.

@splendido
Copy link
Member

...before publishing any of the three I'd like to add some basic tests (with panthomjs?)
basically to tests that exports work fine

...and perhpas some automated streamed publish to get a number of previous release published all at once and not just the latest one.

@Siyfion
Copy link
Author

Siyfion commented May 18, 2015

Sounds good @splendido, I'm wrapping up for the day here now (UK-based) but I'll take a look first thing tomorrow!

@splendido
Copy link
Member

I've just pushed some more stuff to jspdf-core-wrapper to run tests...
...it seems to work fine :-)

The only thing is that on meteor build machines there's no npm, nor phantomjs installed :(

I think I'll try running this stuff on a droplet of mine on DO, but if we think this is the right direction, we should ping MDG to see whether there's room to update VM images to come with npm and phantomjs out of the box...

Please let me know your impressions: does all these grunt tasks make any sense?
...it seemd an easy road to me, but perhaps we can have a simpler solution?

Going to bed also here in Italy ;-)
Cheers

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido can you take a look at the first commit I've made, see if I'm headed in the right direction?
Also, it seems as though there's a bug in the Snap.svg repo, the tagged release "v0.4.1" has the version as "0.4.0" in the bower.json file, I'm trying to get that resolved.

@splendido
Copy link
Member

Seems good!
I'm going to test it right now :-)

the only thing is I'd like to put in the hook field only after we got the webook set up on the upstream repo.
But this is still to be all documented and agreed, and you had no chance to interpret my thoughs ;-)
what about changing it to _hook for now?

The idea is, when such a repo will be enabled on autopublish.meteor.com to check for the hook field in order to decide whether to simply rely on the webhook or put the package in a poll list and manually check for new releases periodically (once a day? more frequently? less?)

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido Cool, I've updated the "hook" field to "_hook" for the time being.

@splendido
Copy link
Member

It worked!

But I've just noticed that you're not using api.export: this is usually bad!
...I've learned it myself with underscorestring:underscore.string :-(
Basically it seems to work if you add the package to your app, but if the package is used/implied form inside another package the game doesn't work anymore!

Could you try something on these lines
This is the same what we're doing here for underscore.string
Lets try to use module.exports like this

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido So can I just copy the way that does it, or is that module = {}; exports trick unique to underscore.string?

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

ie. like this: MeteorPackaging/snapsvg-wrapper@9f38ee7 ??

@splendido
Copy link
Member

well, a copy/paste might be enough, but it really depends on how the library exports the Snap variable.
putting stuf into module.exports is the npm way...

Les try to dig a bit into the code, or simply do some console.dir of modules and export in the post-meteor.js file.

@splendido
Copy link
Member

...at this point I'd say, if tests works for your latest commit all is fine! :-)

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido Hmm, nope, I think my tests started failing since I did that exports stuff... Just going to have some lunch then I'll get back to looking at it.

@splendido
Copy link
Member

if it's a client-side lib, window might be a better bet...

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido That seemed to do the trick. So what's left for me to do...? Anything?

@splendido
Copy link
Member

If we think the approach is valid, we could publish the package.
...I was reasoning on the possibility to publish not just the latest version but also a number of previous versions: possibly in some programmatic way rather than manually?

@Siyfion
Copy link
Author

Siyfion commented May 19, 2015

@splendido Yeah, I guess it would be nice to somehow publish all the versions available. However, for most people I imagine that's surplus to requirements...

@Siyfion
Copy link
Author

Siyfion commented May 21, 2015

@dandv @splendido Is there a recommended naming convention for how we should be publishing these packages? I mean, it'd be good to tell at a glance which are auto-updating and which aren't..?

@Siyfion
Copy link
Author

Siyfion commented May 21, 2015

I've put it here for the time being: https://atmospherejs.com/snapsvg/core

I've also given it a quick test, all seems to be working perfectly... So now for the hook bit, eh @splendido?

@splendido
Copy link
Member

to set it up for autopublish, we need meteorpublish user to be added to the snapsvg organization.
We should also figure out a standard PR text (on the lines of this one maybe...) to explain to the original author what we're trying to achieve and convince them to set up a webhook for us...

Do you guys think the hook creation could be done manually or should we provide a button on autopublish.meteor.com to simply setup the hook without enabling the repository for publishing (as it is now for Meteor packages)?

I'm thinking to start a refactor of the code to end up showing three different toggle buttons.

  • enable auto-publish (for repositories containing a Meteor package)
  • simple webhook creation (for 3rd party library we're going to wrap)
  • enable auto-publish for repo with wrapped packages (which is: repository being under MeteorPackages and including a autopublish.json file..)

does this make any sense?
cc: @chip, @dandv

@splendido
Copy link
Member

@dandv
Copy link
Contributor

dandv commented May 22, 2015

Is there a recommended naming convention

I've always used the official package name if available, to distinguish from random users repackaging the package under their own namespace.

Also, the package description has often included "(official)", which hopefully we'll be able to discard in the future once people stop just wrapping 3rd party libraries, but was instrumental in distinguishing the official Font-Awesome integration from, for example, @linto's https://atmospherejs.com/linto/fontawesome (which for some reason still hasn't been unmigrated sigh)

@Siyfion
Copy link
Author

Siyfion commented May 22, 2015

@splendido Right, meteorpublish has been added to the organisation for snapsvg and snapsvg:core is the package name (that seems to go along with what @dandv was saying about naming).

@splendido
Copy link
Member

👍

I'll try to work a bit on the autopublish side the next days...

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

No branches or pull requests

3 participants