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

Publish Sizzle to npm #266

Closed
wants to merge 2 commits into from
Closed

Publish Sizzle to npm #266

wants to merge 2 commits into from

Conversation

bevacqua
Copy link

@bevacqua bevacqua commented Jun 7, 2014

Defined an entry point so that sizzle can be browserified

@bevacqua
Copy link
Author

bevacqua commented Jun 7, 2014

Maybe I should've used the dist version, to get the right license information and whatnot?

@timmywil
Copy link
Member

timmywil commented Jun 9, 2014

That's right. It should be the dist version.

@bevacqua
Copy link
Author

bevacqua commented Jun 9, 2014

@timmywil fixed. Also, is sizzle published anywhere on npm? All I could find was https://www.npmjs.org/package/sizzle by @rvagg, but it hasn't been updated in years. Maybe he could take it down and you could publish?

I'm sure he'd be fine with it if the official repo gets published in its place

@timmywil
Copy link
Member

timmywil commented Jun 9, 2014

That'd be fine with me.

@rvagg
Copy link

rvagg commented Jun 18, 2014

@ded @ryanve do you want to fill in some context here re Ender and see if an Ender bridge is something that they can accommodate so we can pass off the sizzle package in npm?

I personally don't know anyone using it as the selector engine for Ender but it's obviously in use in a bunch of places:

sizzle downloads

So if we can transition over to an official release while maintaining Ender compatibility and making Browserify happy then it'd be win/win/win.

@timmywil
Copy link
Member

@ded @rvagg jQuery would like to publish the official sizzle repo on NPM under its name. Please drop the sizzle npm package and rename it to something more congenial ("ender-sizzle" perhaps). We know this will break some things for a few people, but it's for the best.

@ded
Copy link

ded commented Jun 26, 2014

i think the browser + ender compat should remain intact. it's not difficult to do, and there's no reason to break things for anyone.

@bevacqua
Copy link
Author

@ded could you guys point out which changes are needed to make sizzle Ender-compatible? I'll make the changes in this branch and everyone wins

@bevacqua
Copy link
Author

Would following these guidelines be enough?

Doing something like src/ender.js

var sizzle = require('./sizzle');
ender.ender(sizzle);

and adding the ender property to the package.json?

@timmywil
Copy link
Member

@ded There are other issues besides the difficulty of supporting ender, politics not withstanding. It really comes down to 2 options. We can either drop a package that hasn't been updated in 2 years and publish Sizzle under the name "sizzle", or you force us to publish the official Sizzle under a different name.

@rvagg
Copy link

rvagg commented Jun 26, 2014

jquery-sizzle is still free in npm but you'd better get in quick!

@bevacqua
Copy link
Author

Not sure why it's that hard to support Ender?

@timmywil
Copy link
Member

Not sure why it's that hard to support Ender?

I meant the difficulty (or lack thereof) is not really a factor.

@bevacqua
Copy link
Author

Exactly. It's not politics, it's not difficult, so why not?

@timmywil
Copy link
Member

Well, no, it is somewhat political. There's more involved when adding this sort of thing. If Sizzle does it, then sooner or later maintainers for other jQuery projects will be asked why they do not also support Ender. Consistency across our projects has become a quality to which we aspire. I am saying that the Ender bridge will not be added because I can predict that even if the suggestion went through the proper channels (i.e. a ticket or forum discussion), it would get shot down. That leaves us with the 2 options I laid out.

@rvagg
Copy link

rvagg commented Jun 26, 2014

Given the excessive politicisation of jquery, I'd suggest an alternative path that avoids interacting with them. @bevacqua I can give you commit access to the repo that maintains our fork and you can keep it in sync with what you need from here and maintain then ender bridge and whatever else you want for Browserify support and we'll publish with the frequency you need.

@bevacqua
Copy link
Author

I'd rather not have to maintain a fork, but I guess unless a jquery-sizzle or similar official package is published there isn't much of an alternative. (Note I don't need Ender, I was just trying to get the conflict resolved)

@mikesherov
Copy link
Member

As @timmywil points out, Ender support would possibly be shot down in discussion. However, I'm open to discussing it anyway, as discussing compatibility pain points usually ends up in interesting solutions regardless.

However... that discussion is immaterial to whether or not the sizzle npm package name is transferred or not. Also, not transferring it makes Ender support a harder discussion to have, as this leaves the name as some sort of weird leverage.

I'm not sure what demonstrates an "excess politicisation of jquery", but it seems like there is an easy and straightforward way to resolve this.

@timmywil
Copy link
Member

Wow, that's a bit of an overreaction. jQuery wants to do what is best for our users. I'm sorry to offend the ender guys, but I personally don't think an ender bridge should be added to Sizzle (this is just me talking by the way, not jQuery as a whole). That said, I do think we should support Browserify. I would like to register Sizzle under its own name, but I can see I have offended the people who could make that possible. Sorry about that.

Nevertheless, I'm not sure why there so much reluctance. Your repo hasn't been updated for a long time and I suspect the majority of those downloading the package are not ender users. Why not switch it to ender-sizzle to make it explicit and avoid the confusion?

@timmywil
Copy link
Member

@bevacqua Rest assured I want to publish to npm regardless of how this conversation turns out. If we're stuck with another name, it will be confusing for the developer community, but so be it.

@timmywil
Copy link
Member

@ded @rvagg I propose we work together. I apologize for not showing concern for existing ender + sizzle users. However, what if we had a way to ensure that nothing broke for anyone, but the official sizzle could still be published to npm as "sizzle".

In other words, since the existing package hasn't been updated for a while, we could leave all of the existing versions untouched. That way, nothing will break for anyone. But the latest versions that have not yet been published could be published with the warning that the ender bridge is no longer present. We can then direct anyone who is updating and still requires the ender bridge to the ender-sizzle package.

What do you think?

@timmywil
Copy link
Member

@ded @rvagg Any thoughts on my proposal?

@ded
Copy link

ded commented Jun 30, 2014

yeah that would be great. i trust the jQuery team as a whole would want things to work. why don't you send me your npm username and point to a commit where the work is being done and then move forward from there

@timmywil
Copy link
Member

@ded Thank you. The npm username that needs to be added with npm owner is "jquery". Beyond that, there's not much work to be done. We'll add a note to the Sizzle README concerning the ender bridge and then publish Sizzle as version 2.0. We can leave the existing packages on npm and Sizzle won't publish versions earlier than 2.0, to avoid any automatic updates causing conflicts.
Thanks again.

@bevacqua
Copy link
Author

This sounds great guys, thanks for dealing with the politics on this one (to both parts)!

@timmywil
Copy link
Member

@ded Oh, as far as re-publishing sizzle with the ender bridge under a different name, I think that's as simple as changing the name in the package.json (that is, in the ender fork's package.json) and running npm publish. We'll provide a direct link to that package on Sizzle's README.

@scottgonzalez
Copy link
Member

The npm username that needs to be added with npm owner is "jquery"

We really shouldn't be using shared accounts.

@timmywil
Copy link
Member

@ded @scottgonzalez In that case, make it "timmywil". I can add other jQuery team members later.

@ded
Copy link

ded commented Jul 1, 2014

Cool. When I get a free moment at my desktop I'll add timmywil
As another reminder, the general quick todo list:

  • bump major version
  • keep in mind common js (and if you want, AMD too). The commonjs will help bundlers like ender, browserify, requirejs, etc.
  • a bridge would be nice, which is a small integration file

@timmywil
Copy link
Member

timmywil commented Jul 1, 2014

Thanks @ded. And thanks for the list.

Just so you know, Sizzle already supports AMD and CommonJS-like require.

@timmywil
Copy link
Member

timmywil commented Jul 1, 2014

Major version has been bumped.

@bevacqua
Copy link
Author

bevacqua commented Jul 1, 2014

Now merge this one in! :)

@timmywil timmywil changed the title Befriend browserify Publish Sizzle to npm Jul 1, 2014
@timmywil
Copy link
Member

timmywil commented Jul 1, 2014

@bevacqua Actually, the main property has already been set. That change made it in with a batch of changes to the package.json. This issue then became the issue to track publishing to npm.

@ded
Copy link

ded commented Jul 1, 2014

ok you should be good to go

@timmywil
Copy link
Member

timmywil commented Jul 1, 2014

Sizzle 2.0 has been published. Thank you everyone!

Release notes: https://github.com/jquery/sizzle/releases/tag/2.0.0

@timmywil timmywil closed this Jul 1, 2014
@bevacqua bevacqua deleted the patch-1 branch July 1, 2014 20:36
@bevacqua bevacqua restored the patch-1 branch July 1, 2014 20:36
@JamesMGreene
Copy link
Member

🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants