ckeditor / ckeditor-boilerplate

A boilerplate for modern git based projects
Other
6 stars 4 forks source link

Boilerplate review #4

Open Reinmar opened 9 years ago

Reinmar commented 9 years ago

While reviewing http://dev.ckeditor.com/ticket/12721 I made a short list of things that we could improve:

Reinmar commented 9 years ago

As for the last point - the README.md could consist of two parts - description of the boilerplate and how to use it and a part to merge into inheriting project's README.

fredck commented 9 years ago

I miss an ability to check a dir or path using the jscs and jshint tasks. In other words I want to be able to execute: grunt jscs core/.

Doesn't sound like a mandatory feature and should not be a blocker. A new issue should be opened for it for future enhancement.

I think that we could keep JSCS and JSHint configs in the root dir (the same way as they are now in https://github.com/ckeditor/ckeditor-dev). They will be easier to find there and this is the standard location and file names so perhaps some IDEs or other tools can benefit from that automatically.

I really want to avoid having the root bloated. It already has lots of files and my idea was leaving there just the files that will be touched by every project. Unfortunately just .editorconfig stayed there because there is no other way for it.

JSCS and JSHint config files shouldn't be touched, ideally, as all projects will be following the same standards. So it looks like ok to have them in the "protected" place they are right now.

As for IDEs, it may be a pity to not have this automated, but at least with mine I'm able to configure the project to read it from a different location. Hopefully the benefit of having a less bloated root compensates it.

The thing that I believe we can't do is to keep ignored files lists as they are now in https://github.com/ckeditor/ckeditor-dev, because: a) we want to share the list, b) we want to extend them with .gitignore.

I don't understand this. Right now we're using .gitignore only.

The jscs and jshint can share the list of ignored files - it's hard to think of a realistic case in which these lists should differ.

Right now they already do this. It's the .gitignore file.

The API docs style should be fixed.

If you mean anything more than this, please feel free to push fixes directly.

The README.md should contain a more detailed description of available Grunt tasks, focused on contributors. I want to merge this lists of tasks into other projects' READMEs to expose them to other developers, so this description should be written for the end-project developers. In short - DRY.

I think the way it is described right now is more than sufficient. It's concise.

I understand your point though, but we're really taking about a small part of the README file, while all the rest would be dropped. I think that most of the people would not even consider using it because of this.

One idea would be having a template for README in the wiki, having the main README point to it.

Reinmar commented 9 years ago

Doesn't sound like a mandatory feature and should not be a blocker. A new issue should be opened for it for future enhancement.

I reported #5 and #6.

I really want to avoid having the root bloated. It already has lots of files and my idea was leaving there just the files that will be touched by every project. Unfortunately just .editorconfig stayed there because there is no other way for it.

JSCS and JSHint config files shouldn't be touched, ideally, as all projects will be following the same standards. So it looks like ok to have them in the "protected" place they are right now.

As for IDEs, it may be a pity to not have this automated, but at least with mine I'm able to configure the project to read it from a different location. Hopefully the benefit of having a less bloated root compensates it.

Fair enough :).

Right now they already do this. It's the .gitignore file.

Here you have them twice: https://github.com/ckeditor/ckeditor-boilerplate/blob/master/gruntfile.js. And project like CKEditor has some assets which should not be checked, so physically I need to add them to both these arrays. I can reuse one array and that's exactly what I meant - there should be only one array defined in that file. It's a detail of course.

I think the way it is described right now is more than sufficient. It's concise.

It's good if you already know how it works, but I don't think it's enough for other developers who may not be familiar with Grunt and git hooks. So it's ok for boilerplate's README, but projects' READMEs should be written from other perspective. I will need to write something for ckeditor-dev, so I'll put it in the wiki.

Reinmar commented 9 years ago

I'll address the remaining points (mainly API docs and the wiki article) and close this ticket.