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

Establish criteria and process to review middleware listed on the site #988

Open
crandmck opened this issue Oct 13, 2018 · 12 comments
Open

Comments

@crandmck
Copy link
Member

http://expressjs.com/en/resources/middleware.html#additional-middleware-modules lists some middleware modules that are note maintained by the Expressjs team.

We get requests to add more modules to this list (see #968 and #981 for example), but as @wesleytodd noted:

Adding to the website is an explicit recommendation to the community, and I think we should be judicious about that.

So, I suggest we

  1. Establish some basic criteria for listing middleware in the above page.
  2. Review the existing list to ensure that all the modules meet those criteria.
  3. Add a brief explanation of the criteria and process to http://expressjs.com/en/resources/contributing.html (or elsewhere, as appropriate).

Does anyone have suggestions for basic criteria for num. 1? @expressjs/expressjs-com

@dougwilson
Copy link
Contributor

So this has been a similar issue for the express-session module, as it relies on folks implementing store plugins to be useful to the general population. Of course, actually vetting some kind of coding quality or something for things to be listed seemed to much of a burden for both sides (our side and the author side), and so the current criteria for express-session is just (a) it works with the current version of the module.

So to me, that comes back around to the Express middleware modules. I think there are a lot of useful middleware out there, even serving a purpose perhaps none of the Express team is familiar with to really know if it's "good" or not. Along the lines of the express-session criteria I would put forth the following initial criteria set for middleware we list:

(1) It works with the current version of Express
(2) It is not known to cause undue interference to the normal Express operations and case interoperability issue with other Express middleware

Not sure what else to really add that is going to be easy to "police".

@brandon-d-mckay
Copy link

brandon-d-mckay commented Oct 16, 2018

@dougwilson Something easy to add would be the package quality rating/score determined by npmjs.com/npms.io (although I think the latter may no longer be updating its results... the scores of my package (krauter =]) are updated on npmjs.com but not on there). A score of 90%+ can almost guarantee that the package at least has a readme, license, tests, linting, up-to-date dependencies, and a proper package.json.

If anyone's up for it, we could also modify the source code they use (link here), and add our own criteria.

@dougwilson
Copy link
Contributor

Hi @brandon-d-mckay I think that can be a good idea. If we do that, we'd probably want to be able to explain to people what exactly they need to do to achieve a 90%+ (or whatever score). I cannot find where this score is located on npmjs.com, but I do see them on npms.io. It seems even our "official" middleware does not meet the 90%+ criteria proposed here. We probably should either (a) not require others to meet a score even out own module do not meet or (b) get our module scores raised before instituting the requirement.

Examples: morgan=86%, serve-favicon=85%, cors=88%, express-session=71%, cookie-session=76%, serve-static=89%, etc. I stopped looking at this point, as I haven't found any of our middleware with >= 90%

@wesleytodd
Copy link
Member

In spirit I think this is the right direction. In practice I think we will struggle to get it right.

I think a combination of analysis tools and manual judgement is a better approach. For example, by the metrics in npms, a package which fulfills a niche use case will never go above 90% because its popularity score will be low. But it might be just the right solution for a user in their case, and a great fit for our recommendation. Similar to niche use cases, new package will suffer the same way, low popularity score despite being overall high quality.

@brandon-d-mckay
Copy link

Oh, I was thinking it'd only be the "Quality" score, not the overall one. Like morgan has 97% quality (86% overall), cors has 99% quality (88% overall), and express-session has 94% quality (71% overall).

@dougwilson
Copy link
Contributor

Ah, gotcha. Where do you find that on npmjs.com (or is this from npms.io)? And say for example morgan had 89%, and thus would not qualify, where do we point the author to to understand ho to achieve at least 90% (i.e. why, exactly, that given module is below 90%)?

@brandon-d-mckay
Copy link

brandon-d-mckay commented Oct 16, 2018

On npmjs.com, I think you can only see the scores when viewing a package among search results, and not on the actual page for the package. But there is a public search API (although it has pretty limited options). You could search the package name as a keyword and then also constrain the search by package author, which will most likely give you the single result you want.

Apparently you can also replicate the registry database, which I believe is what npms.io is doing. The outdated information on their site may be because they haven't replicated/updated their version of the database lately.

And there's also an API for download counts.

EDIT: https://github.com/local-npm/local-npm (could be useful)

@dougwilson
Copy link
Contributor

Nice @brandon-d-mckay ! Say for example morgan had 89%, and thus would not qualify, where do we point the author to to understand how to achieve at least 90% (i.e. why, exactly, that given module is below 90%)?

@brandon-d-mckay
Copy link

brandon-d-mckay commented Oct 16, 2018

https://npms.io/about

But then again, this all does require a lot of initial work to setup. Might be easier to just have an "official" module/middleware list, along with a community one that just requires basic approval/review and comes with some kind of "as-is" disclaimer. And perhaps issues could be opened (either by public users or expressjs organization members) for any module listed on the community page that is suspected to be "unworthy" or of low quality.

@crandmck
Copy link
Member Author

crandmck commented Oct 17, 2018

Good discussion.

Might be easier to just have an "official" module/middleware list, along with a community one that just requires basic approval/review and comes with some kind of "as-is" disclaimer.

We have this to some degree now.... The modules listed in the http://expressjs.com/en/resources/middleware.html#express-middleware section basically lists the "official" ones and the others under "Additional middleware modules" are the caveat emptor ones (except we don't have a disclaimer).

I agree it would be far simpler to go this route... and just state that the other middleware modules have not been vetted in any way (except perhaps to just check that they are actually available on npm - that seems like a minimum). But it certainly seems like there would be value to the community in vetting some modules beyond that. The question is: do we want to go down that road?

If not, then we can just put a big disclaimer above the "Additional middleware modules" and then list any ones that are submitted and we confirm are on npm. If we do, then there's a lot more work to do. Thoughts?

@hsluoyz
Copy link

hsluoyz commented Feb 5, 2019

Any update on this issue?

@crandmck
Copy link
Member Author

This fell by the wayside, but I think it bears discussion at the next TC meeting.
@expressjs/express-tc @expressjs/expressjs-com

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

5 participants