Skip to content
This repository has been archived by the owner on Aug 6, 2021. It is now read-only.

Adding all possible model settings to template files #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding all possible model settings to template files #9

wants to merge 1 commit into from

Conversation

joeljuca
Copy link

I've added all possible model settings to template files as comments, alongside small descriptions. This would be really useful for beginner developers jumping on Sails.js, as well as experienced developers that needs to check documentation pages to define that specific setting to change a specific behavior of Sails but can't remember the name.

I tried to fix the identation style used in the generated template (I could understand why its that way, since I spent three hours with it trying to reach a perfect output for models). I couldn't figure out how to force the CoffeeScript output, so I couldn't test it - but I believe its working.

Also, I removed the @description comment tags that contains TODOs, since there's no need to have them, and IDEs that provide support for TODO comments will get filled with unmeaning entries (it's strange but there are people using it). That's not good.

I'm bundling multiple changes into one PR but since it does not add/change/remove functionality and, in practice, I'm just inserting documentation on comments, I though it would be a waste of time if I split these changes on some PRs.

@joeljuca
Copy link
Author

Sorry for my poorly written English :( its 4:00 AM

@konstantinzolotarev
Copy link
Member

@joelwallis
I'm not sure it's a good way to create all this comments for every model.

Might be it will be better to add some flag for model creation ? Like
sails generate model --full Some and only with this flag it will create all this comments.
I think it will add flexibility and for users who working with sails.js a lot model will remain clean.

What do you think about this ?

Hey @sgress454 @tjwebb you are welcome to join conversation ;)

@joeljuca
Copy link
Author

@konstantinzolotarev well, it sounds interesting, but providing the easy way through a CLI param does not really keep it as the easy way. Beginners will not know the optional parameter to generate clean models.

Instead, what you think about providing a global setting which, when turned on, will generate clean models instead of this heavily documented one? This will keep it easy to use for beginners while experienced developers would be able to place a one-time configuration that will avoid unnecessary documentation comments.

@konstantinzolotarev
Copy link
Member

Yep it could be added. Something that could be set up into config/models.js

@joeljuca
Copy link
Author

@konstantinzolotarev I'll figure out how I should do this and update this PR.

@sgress454
Copy link
Member

@joelwallis thanks for the contribution! @konstantinzolotarev is right; this is a bit heavy for regular use. A good compromise might be having it available via a command-line flag, and putting information to that effect in the header of the generated model files, e.g. a comment at the top that says, "to generate a model with all options available, do sails generate model --full myModel".

@joeljuca
Copy link
Author

Well, I'm very used with other frameworks that do exactly the opposite, providing commented configuration options alongside documentations, which streamline the beginner experience. But yeah, having a CLI flag is still better than having no way to have it. :)

I need to work on it. Any insights about how to get sensible to CLI flags when generating models?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants