MeteorPackaging / discussions

Ask for Meteor integration help, or discuss Meteor 3rd party library packaging
5 stars 1 forks source link

Snap SVG #13

Open Siyfion opened 9 years ago

Siyfion commented 9 years ago

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

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

splendido commented 9 years ago

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

Hope everything is fine!

Siyfion commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

splendido commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

...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 commented 9 years ago

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

splendido commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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

splendido commented 9 years ago

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 commented 9 years ago

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

Siyfion commented 9 years ago

ie. like this: https://github.com/MeteorPackaging/snapsvg-wrapper/commit/9f38ee71c61402e272563373b9a0debb4686d3ed ??

splendido commented 9 years ago

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 commented 9 years ago

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

Siyfion commented 9 years ago

@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 commented 9 years ago

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

Siyfion commented 9 years ago

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

splendido commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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.

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

splendido commented 9 years ago

See also https://github.com/MeteorPackaging/autopublish.meteor.com/issues/20

dandv commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

:+1:

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