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

Concept version of a CONTRIBUTING.md #798

Merged
merged 2 commits into from
Aug 7, 2016
Merged

Concept version of a CONTRIBUTING.md #798

merged 2 commits into from
Aug 7, 2016

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Jul 21, 2016

First concept version of CONTRIBUTING.md

  1. If you want to make a change, like different titles, paragraph order, etc. Then please let me know here, or in irc. And i'll change it.
  2. If you want to add a chapter. For example, some more in depth Javascript guidelines, please add them yourself.
  3. If you think any grammatical improvements can be made, please make them!

I'll keep this open for 1 week.

  • Read contribution guide
  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review

@ratoaq2
Copy link
Contributor

ratoaq2 commented Jul 22, 2016

Should we add PEP257 as well?
Talking about docstrings, I like to have the docstrings with the parameter types and return types, since they help a lot my IDE helping myself.
What are the opinions from you all?

@medariox
Copy link
Contributor

medariox commented Jul 24, 2016

My suggestion would be that we try to standardize our code as much as possible by forcing new PRs to pass specific conventions.

In my opinion, these should be mandatory:

That's my opinion. What do you think?

@ratoaq2
Copy link
Contributor

ratoaq2 commented Jul 29, 2016

Take a look in this proposal:
#836

@OmgImAlexis
Copy link
Collaborator

I'd love to convert to the JS style guide used at Airbnb but there's still a LOT of changes that'll need to be made before that's possible. I'll finally have internet back in the next 14 days so I'll be able to start work on this if everyone is okay with that. I'd also like to know officially what versions of browsers we support so I can start removing old js code that's mainly for outdated browsers.

One of first things that comes into mind is using const and let vs var which is only supported by modern browsers this would mean we'd need to drop support for IE10 which I'm not sure if we can do. I need to have a look at the logs we've got from cdn.pymedusa.com first to see how many of the users are using anything below IE11.

@p0psicles
Copy link
Contributor Author

Can this be approved?

@fernandog
Copy link
Contributor

fernandog commented Aug 5, 2016

Approved

Approved with PullApprove

@coveralls
Copy link

Coverage Status

Coverage remained the same at 28.874% when pulling 708e7bf on contributing into bf4efd0 on develop.

```

## Javascript guidelines
We follow Google's Javascript guide: https://google.github.io/styleguide/javascriptguide.xml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason Google's was chosen over AirBNB's?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Just want to get starting with something. And as where going towards Angular, googles guidelines seemed logic. If you wanna take care of the js guidelines. Great!

Where now on slack with for the devs channel. Come to IRC for the invite.

@OmgImAlexis
Copy link
Collaborator

For the JS devs I've also been looking into https://github.com/sindresorhus/xo instead of jshint and the current style/lint guide we use.

@p0psicles p0psicles merged commit f90f3d3 into develop Aug 7, 2016
@fernandog fernandog deleted the contributing branch August 21, 2016 11:31
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

Successfully merging this pull request may close these issues.

6 participants