generate / generate-license

Generate a license file from the command line when Generate is installed globally, or use this generator as a plugin or sub-generator in your own generator project.
MIT License
91 stars 10 forks source link

Choosealicense #6

Closed jonschlinkert closed 8 years ago

jonschlinkert commented 8 years ago

Implements all of the licenses from https://github.com/github/choosealicense.com.

positives

negatives

@pointnet this was a collaborative effort between @doowb and I. We'd like your feedback before merging.

pointnet commented 8 years ago

Nice one, I will learn a lot of stuff with all that code!!

On the other hand, I think you are not taking the right path regarding it, let me explain you my point of view.

The code is too much complex for that little task and using gulp in an base/assemble/generate project seems me so weird :smile:

I also think we don't need that cache.json file, this is just more complexity because we don't need to generate the templates, it's nice for "cold start" but should not be integrated as part of the whole project.

We need to think of that on a more generic point of view, the goal here is:

This is something we need in a lot of generators and could be seen as some kind of "build tool".

We can generate a generator.tasks.js containing the flatten code based on template metadata as it was in my feature/rework and just use it in the generator.js one.

At the end, we provide a new tool for generators developers, the code will be way more readable and less complex.

My 2 cents, feedbacks?

pointnet commented 8 years ago

Here's a concrete implementation of my idea

https://github.com/generate/generate-license/tree/feature/templates-transform

This implementation is using https://github.com/pointnet/generate-templates-transform

What do you guys think of this approach ? Am I totally dumb ?

jonschlinkert commented 8 years ago

We can generate a generator.tasks.js containing the flatten code based on template metadata as it was in my feature/rework and just use it in the generator.js one.

I think this is exactly what @doowb was saying! good idea. I love it. @doowb?

doowb commented 8 years ago

Sorry for not replying sooner...

I think this is exactly what @doowb was saying!

This is what I was thinking.

@pointnet I like how you did this and that you created another generator to handle loading and rendering the templates.

Doing it this way, we can generate the readme using the code comments from generate.tasks.js and the tests and startup time should speed up again.

pointnet commented 8 years ago

@doowb that's the idea, dynamic tasks should be used for what they are named for => dynamic stuff

I'm not sure my implementation is the best one, maybe you guys have other ideas of how we can implement that ?

jonschlinkert commented 8 years ago

I'm not sure my implementation is the best one

It's better than what we have! want to merge it in?

jonschlinkert commented 8 years ago

@pointnet are you still interested in possibly merging that code in, or doing a pr? no pressure just curious, it looks great

pointnet commented 8 years ago

Yeah for sure, it's just I'm a bit overloaded with my daily job right now and still thinking of the best way to implement it.

Today, my proposal is doing two things, first it extracts template metadata from a template glob and then it uses those data to generate code based on a template. All is done in one plugin.

In term of separation of concerns, this is not good, I think the best way is to create two plugins

Doing that way, we will be able to use template metadatas for other stuff than what I made in generate-license and this could make more sense.

Any inputs, advices are welcome :smile:

jonschlinkert commented 8 years ago

Good idea. I like the approach. I think we can use to generate tasks for .gitignore templates as well, from https://github.com/github/gitignore. This will be really nice to use!

it's just I'm a bit overloaded with my daily job right now

no worries! I understand completely

jonschlinkert commented 8 years ago

closing since we implemented something close to this. there is still room for improvement, but this works for now