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

Community Maintainer Guidelines #223

Closed
strandedcity opened this issue Jan 14, 2017 · 5 comments
Closed

Community Maintainer Guidelines #223

strandedcity opened this issue Jan 14, 2017 · 5 comments
Labels
documentation Changes only affect the documentation

Comments

@strandedcity
Copy link
Contributor

If we're going to start having more people involved in Jimp's development, it probably behooves the community to have clear answers for common questions like:

  • If I have a question, what do I need to do to get an answer?
  • When can an issue / PR be closed?
  • When making pull requests, what are code style and syntax guidelines to follow (there are several active issues about ES2015 / typescript / .editorconfig / coffeescript right now)
  • What are the testing requirements for PR's to be merged
  • Automated testing?
  • Jimp conventions & clear development priorities
  • Running list of bugs to be fixed and desired features for development

Additionally I think some conventions might be useful. For example, I think we're starting to have a need for "optional" parameters to be passed into some Jimp function, and other Jimp functions which accept so many parameters that having them in a specified order becomes confusing. Mrdoob of threejs draws the line at three, but the terrific work in https://github.com/oliver-moran/jimp/pull/191/files implements a function with seven:

Jimp.prototype.composite = function (src, x, y, mode, opacitySource, opacityDest, cb)

Should that be this instead?

Jimp.prototype.composite = function (src, options, cb)

where options is an object with defaults like:

{
   x: 0,
   y: 0,
   mode: Jimp.BLEND_SOURCE_OVER,
   opacitySource: 1,
   opacityDest: 1
}

?

All just food for thought.

@aurium
Copy link
Contributor

aurium commented Jan 18, 2017

Automated testing? Sure! On my ToDo to start and mandatory to all new PRs.

I like that! Jimp.prototype.composite = function (src, options, cb)

@iwsfg
Copy link
Contributor

iwsfg commented Jan 18, 2017

If we're gonna have options objects we should probably provide extensive information for autocompletion engines. Does anyone know how many of those can use .d.ts files as information sources?

Maybe we wont even need documentation comments anymore. But in case we still do - we should probably normalize those and mention that in contributor guidelines. Right now most of pull requests comes with proper JSDoc3 annotations, while existing documentation comments written in some other format, which I don't recognize, and they don't provide any type information.

I cant rewrite those into proper-ish JSDoc comments.

@iwsfg iwsfg added the documentation Changes only affect the documentation label Jan 20, 2017
@iamropo
Copy link

iamropo commented Feb 14, 2017

We should follow a standard JavaScript style guideline to avoid conflicts, preferably http://standardjs.com/

@iwsfg
Copy link
Contributor

iwsfg commented Feb 15, 2017

"standard" style guideline

No thanks.

@iwsfg
Copy link
Contributor

iwsfg commented Feb 15, 2017

Perhaps, I should be more clear. That was in regard of npm://standard. I don't like the way it's being presented. There's no standard guideline. The fact that Isaac and some other guy on the web like that particular code style doesn't make it standard. The fact they have fancy name for their package doesn't make it standard either.

We have configured ESLint on test branch though, it works just fine and doesn't require existing code style to be changed all that much. And existing code is pretty consistent in its style too, so why change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Changes only affect the documentation
Projects
None yet
Development

No branches or pull requests

5 participants