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

Coding style conventions #2

Open
gillesfabio opened this issue Jul 19, 2014 · 18 comments
Open

Coding style conventions #2

gillesfabio opened this issue Jul 19, 2014 · 18 comments

Comments

@gillesfabio
Copy link
Contributor

Some files are indented with tabs, some with spaces, some with a mix of both... Please, add a CONTRIBUTING file or Contributing section in README with coding style conventions to use and EditorConfig support.

@acemtp
Copy link
Owner

acemtp commented Jul 19, 2014

Good idea.

Let s follow the meteor guide style

https://github.com/meteor/meteor/wiki/Meteor-Style-Guide

Le 19 juil. 2014 à 10:16, Gilles Fabio notifications@github.com a écrit :

Some files are indented with tabs, some with spaces, some with a mix of
both... Please, add a CONTRIBUTING file or Contributing section in README
with coding style conventions to use and EditorConfig
http://editorconfig.org support.


Reply to this email directly or view it on GitHub
#2.

@gillesfabio
Copy link
Contributor Author

So code should be reformatted properly.
I sent two PRs: #3 and #4 that improve README and add EditorConfig support.

@LeCoupa
Copy link
Contributor

LeCoupa commented Jul 19, 2014

Yes, Meteor-Style-Guide is great! I have updated the wiki. Let's write our conventions there when something is different than the Official Meteor Style Guide.

  1. Should we use CoffeeScript? (I'm a huge fan, but I know that many people are reluctant to it)
  2. Should we use Jade & SASS? (to make our code more clear and organized)

@gillesfabio
Copy link
Contributor Author

  1. Everyone (any Meteor developer) knows JS, so choosing JS won't restrict contributions. CoffeeScript is purely awesome, BTW. Harmony too. Personally, I don't mind.
  2. SASS? Why not. Jade? Like CoffeeScript. Sounds a bit out-of-context. The source code of Meteor France can be a learning base for newcomers (reading its source code). That would be cool to stay "common". Jade is awesome, BTW. I don't mind.

@LeCoupa
Copy link
Contributor

LeCoupa commented Jul 19, 2014

Okay, I agree, let's make it a learning base for newcomers: JavaScript and pure HTML sound good. If we could use SASS that would be great, but I can adapt if people prefer SCSS or LESS.

@gillesfabio
Copy link
Contributor Author

I confess, I have some love for SASS too. Maybe we could vote? Nine contributors. Ten watchers. That would be great to have their opinion. ping @acemtp @fabien-h @adrienjoly @Guillaume-dev @williamledoux @davidbourguignon @xgorse @dadge @pinouchon

@adrienjoly
Copy link
Collaborator

I never used sass. If you think that it's very common to use in the meteor world, and if it's easy to migrate our code, I'm ok with it.

Adrien Joly (mobile)
http://adrienjoly.com/

Le 20 juil. 2014 à 10:13, Gilles Fabio notifications@github.com a écrit :

I confess, I have some love for SASS too. Maybe we could vote? Nine contributors. Ten watchers. That would be great to have their opinion. ping @acemtp @fabien-h @adrienjoly @Guillaume-dev @williamledoux @davidbourguignon @xgorse @dadge @pinouchon


Reply to this email directly or view it on GitHub.

@gillesfabio
Copy link
Contributor Author

@adrienjoly Thanks. Honestly, I don't know if it's very common in the Meteor ecosystem. Just saw it's already in use on the project and has some love. And what about JS vs CoffeeScript?

@adrienjoly
Copy link
Collaborator

I vote against coffeescript because meteor is javascript based and coffeescript is not javascript anymore => people that are not comfortable with Cs will not be able to understand the meteor project's code => we would not satisfy our goal of sharing a project for learning purposes (or at least it would not target beginners anymore)

Adrien Joly (mobile)
http://adrienjoly.com/

Le 20 juil. 2014 à 10:32, Gilles Fabio notifications@github.com a écrit :

@adrienjoly Thanks. Honestly, I don't know if it's very common in the Meteor ecosystem. Just saw it's already in use on the project and has some love. And what about JS vs CoffeeScript?


Reply to this email directly or view it on GitHub.

@gillesfabio
Copy link
Contributor Author

@adrienjoly Thanks. I agree.

@fabien-h
Copy link
Collaborator

Actually, you don't have to use sass or to migrate existing code if other people are using it in the project. Because when using the scss version, the basic css syntax is valid.

In your scss files, you can write this :

body{color: #333;
    header{color: #666;}
}

Or this :

body{color: #333;}
body header{color: #666;}

The first block will be converted into the second and the second won't be touched. Both are valid and can follow any style guide you want.

But honestly, sass without compas...

The question is more : does the designer wants sass ? Like the bootstrap use, it's not good or bad, it's relevant or not depending on the organisation.

Coffe / js does the same thing. But any coffee user wille be confortable with js. All the js users are not confortable with coffee.

@acemtp
Copy link
Owner

acemtp commented Jul 20, 2014

Sent from mobile so I ll be short.

This app could be a cool app that new people could learn.

So:

1/ it should stay in JavaScript CSS HTML. Even if I really like sass jade
and so on.

2/ it would be nice to have one directory per module (jobs/ meetup/
project/) instead of view/ controller. So if I want to understand how jobs
is coded, I know everything is in job/.

Le 20 juil. 2014 à 11:00, fabien huet notifications@github.com a écrit :

Actually, you don't have to use sass or to migrate existing code if other
people are using it in the project. Because when using the scss version,
the basic css syntax is valid.

In your scss files, you can write this :

body{color: #333;
header{color: #666;}
}

Or this :

body{color: #333;}
body header{color: #666;}

The first block will be converted into the second and the second won't be
touched. Both are valid and can follow any style guide you want.

But honestly, sass without compas...

The question is more : does the designer wants sass ? Like the bootstrap
use, it's not good or bad, it's relevant or not depending on the
organisation.

Coffe / js does the same thing. But any coffee user wille be confortable
with js. All the js users are not confortable with coffee.


Reply to this email directly or view it on GitHub
#2 (comment).

@gillesfabio
Copy link
Contributor Author

@fabien-h @acemtp, thanks.

I agree for SASS. Kind of overkill? So let's stay HTML/CSS/JS? Everyone agrees?

@acemtp, could you tell us more about the "one directory per module"?

client/
client/config/
client/styles/
client/modules/
client/modules/jobs/
client/modules/meetups/

Like that?

The modules folder creates a context to clearly separate client's modules from client's stylesheets, configs and other client's root folders. My client/views PR (#6) is similar:

client/
client/config/
client/styles/
client/views/
client/views/jobs/
client/views/meetups/

Here, views means "template". Just the "V" of MVC. Nothing else. It doesn't need to be systematically coupled to a controller. views and not modules because view files depend on server (publications) and "both" (collections). They are not "modular". So it's kind of a lie to name it modules. It takes sense if we apply this split at project's level, not only for client. The @LeCoupa boilerplate is a perfect example of this approach. Clean. Relevant. If we want to make things the Meteor way, we can even use packages like Fraction but it would be hard to understand for newcomers and would restrict contributions to developers with advanced Meteor skills.

@acemtp
Copy link
Owner

acemtp commented Jul 20, 2014

HTML/CSS/JS for now yes.

Less is more.

client/jobs/
client/meetups/

With so small projects like this one, I really fed up to have to click on 5
directories to finally find one file with 5 lines.

jobs/jobs.js
jobs/jobs.html
jobs/jobs.css

until the files are < 200 lines is completely OK. New people can just open
one file and see the whole code for jobs in the same file. No need to
switch to 10 files of 5 lines trying to find how things works.

Vianney Lecroart

Website http://www.ploki.info | Blog http:/blog.ploki.info |
LinkedIn http://fr.linkedin.com/in/acemtp | Facebook
http://www.facebook.com/acemtp | @acemtp http://www.twitter.com/acemtp

PS: I welcome VSRE http://vsre.info/ emails!

On Sun, Jul 20, 2014 at 1:35 PM, Gilles Fabio notifications@github.com
wrote:

@fabien-h https://github.com/fabien-h @acemtp
https://github.com/acemtp, thanks.

I agree for SASS. Kind of overkill? So let's stay HTML/CSS/JS? Everyone
agrees?

@acemtp https://github.com/acemtp, could you tell us more about the
"one directory per module"?

client/
client/config/
client/styles/
client/modules/
client/modules/jobs/
client/modules/meetups/

Like that?

The modules folder creates a context to clearly separate client's modules
from client's stylesheets, configs and other client's root folders. My
client/views PR (#6 #6) is
similar:

client/
client/config/
client/styles/
client/views/
client/views/jobs/
client/views/meetups/

Here, views means "template". Just the "V" of MVC. Nothing else. It
doesn't need to be systematically coupled to a controller. views and not
modules because view files depend on server (publications) and "both"
(collections). They are not "modular". So it's kind of a lie to name it
modules. It takes sense if we apply this split at project's level, not
only for client. The @LeCoupa https://github.com/LeCoupa boilerplate
https://github.com/Gentlenode/hackathon-meteor is a perfect example of
this approach. Clean. Relevant. If we want to make things the Meteor way,
we can even use packages like Fraction https://github.com/fraction/news
but it would be hard to understand for newcomers and would restrict
contributions to developers with advanced Meteor skills.


Reply to this email directly or view it on GitHub
#2 (comment).

@williamledoux
Copy link
Collaborator

I honestly think that while a specific module fits in one file of each type, we could get rid of the folder, and add the folder once it's needed.

client/meetups.html
client/meetups.js
client/meetups.css
client/apps.html
client/apps.js
client/apps.css
client/jobs/newjob.js
client/jobs/newjob.html
client/jobs/joblist.js
client/jobs/joblist.html
client/jobs/jobs.css

@acemtp
Copy link
Owner

acemtp commented Jul 20, 2014

+1 william

Vianney Lecroart

Website http://www.ploki.info | Blog http:/blog.ploki.info |
LinkedIn http://fr.linkedin.com/in/acemtp | Facebook
http://www.facebook.com/acemtp | @acemtp http://www.twitter.com/acemtp

PS: I welcome VSRE http://vsre.info/ emails!

On Sun, Jul 20, 2014 at 6:16 PM, William Ledoux notifications@github.com
wrote:

I honestly think that while a specific module fits in one file, we could
get rid of the folder, and add the folder once its needed

client/meetups.html
client/meetups.js
client/meetups.css
client/apps.html
client/apps.js
client/apps.css
client/jobs/newjob.js
client/jobs/newjob.html
client/jobs/joblist.js
client/jobs/joblist.html
client/jobs/jobs.css


Reply to this email directly or view it on GitHub
#2 (comment).

@gillesfabio
Copy link
Contributor Author

That's not a LOC or number of files issue. Both approaches have the same amount. It's contextual. Having modules at the same level of everything else in client folder leads to confusion. It's very important to have a clear and well-defined structure for newcomers. I put myself in the shoes of a newcomer and found this:

client/helpers/
client/config/
client/styles/
client/jobs/
client/meetups/
client/layouts/

Or even worse:

client/meetups.html
client/meetups.js
client/meetups.css
client/apps.html
client/apps.js
client/apps.css
client/jobs/newjob.js
client/jobs/newjob.html
client/jobs/joblist.js
client/jobs/joblist.html
client/jobs/jobs.css

I'm sorry but I just think "what a mess! bye bye!".

Now, if I add a single one directory named views:

client/helpers/
client/config/
client/styles/
client/views/jobs/
client/views/meetups/

I can instantly understand the project. If I'm a newcomer coming from Django, Rails, Symfony, Express or any other framework, this structure sounds familiar to me. "Views"? Oh well, I know. When you learn a new technology, having landmarks can help a lot.

@acemtp @williamledoux guys, keep in mind that you're building a project that mainly targets newcomers. Please please, help them to love Meteor. Less is more when you master the tool.

@acemtp
Copy link
Owner

acemtp commented Jul 20, 2014

As a newcomer, I really prefer to see something like:

jobs/client.js
jobs/server.js
jobs/style.css
...

So if i want to understand how jobs are working on the project, I know
that everything I have to read is in jobs/ and I don't have to know that
some part are in server/jobs, another part in views and so on.

And if they want familiar structure, they just can go and see telescope :)

Vianney Lecroart

Website http://www.ploki.info | Blog http:/blog.ploki.info |
LinkedIn http://fr.linkedin.com/in/acemtp | Facebook
http://www.facebook.com/acemtp | @acemtp http://www.twitter.com/acemtp

PS: I welcome VSRE http://vsre.info/ emails!

On Sun, Jul 20, 2014 at 6:29 PM, Gilles Fabio notifications@github.com
wrote:

That's not a LOC or number of files issue. Both approaches have the same
amount. It's contextual. Having modules at the same level of everything
else in client folder leads to confusion. It's very important to have a
clear and well-defined structure for newcomers. I put myself in the shoes
of a newcomer and found this:

client/helpers/
client/config/
client/styles/
client/jobs/
client/meetups/
client/layouts/

Or even worse:

client/meetups.html
client/meetups.js
client/meetups.css
client/apps.html
client/apps.js
client/apps.css
client/jobs/newjob.js
client/jobs/newjob.html
client/jobs/joblist.js
client/jobs/joblist.html
client/jobs/jobs.css

I'm sorry but I just think "what a mess! bye bye!".

Now, if I add a single one directory named views:

client/helpers/
client/config/
client/styles/
client/views/jobs/
client/views/meetups/

I can instantly understand the project. If I'm a newcomer coming from
Django, Rails, Symfony, Express or any other framework, this structure
sounds familiar to me. "Views"? Oh well, I know. When you learn a new
technology, having landmarks can help a lot.

@acemtp https://github.com/acemtp @williamledoux
https://github.com/williamledoux guys, keep in mind that you're
building a project that mainly targets newcomers. Please please, help
them to love Meteor. Less is more when you master the tool.


Reply to this email directly or view it on GitHub
#2 (comment).

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

6 participants