search

acemtp / meteor-france

MIT License
7 stars 2 forks source link

Coding style conventions #2

Open gillesfabio opened 10 years ago

gillesfabio commented 10 years ago

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 commented 10 years ago

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 https://github.com/acemtp/meteor-france/issues/2.

gillesfabio commented 10 years ago

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

LeCoupa commented 10 years ago

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 commented 10 years ago
  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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

@adrienjoly Thanks. I agree.

fabien-h commented 10 years ago

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 commented 10 years ago

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 https://github.com/acemtp/meteor-france/issues/2#issuecomment-49541002.

gillesfabio commented 10 years ago

@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 commented 10 years ago

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 https://github.com/acemtp/meteor-france/pull/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 https://github.com/acemtp/meteor-france/issues/2#issuecomment-49544167.

williamledoux commented 10 years ago

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 commented 10 years ago

+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 https://github.com/acemtp/meteor-france/issues/2#issuecomment-49551245.

gillesfabio commented 10 years ago

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 commented 10 years ago

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 https://github.com/acemtp/meteor-france/issues/2#issuecomment-49551645.