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

Naming issue in DSL #29

Merged
merged 2 commits into from
Sep 23, 2013
Merged

Naming issue in DSL #29

merged 2 commits into from
Sep 23, 2013

Conversation

SergeyKishenin
Copy link
Collaborator

I have a remark/suggestion.

Bower is not JavaScript specific. Packages can contain JavaScript, CSS, images etc. So my suggestion is: rename Jsfile to Assetfile and js method to asset. The method syntax gonna be the same as for js libraries also for css ones.

If you agree I'll be back soon with my commit.

@rharriso
Copy link
Owner

What "js" are you talking about renaming?

I don't like the sound of assetfile. Maybe something else though, I do see the value of not using JSFile. But don't use the DSL, So that's just like my opinion man.

@SergeyKishenin
Copy link
Collaborator Author

Here's the "js" I'm talking about renaming: https://github.com/42dev/bower-rails/blob/master/lib/bower-rails/dsl.rb#L43. It was about that calling asset 'bootstrap' is more correct that js 'bootstrap'

I find using DSL a good practice: at least it is more familiar for Ruby developers as it is Bundler-like.

I can suggest Bowerfile instead, like Guardfile, Rakefile (by the name of the lib).

@rharriso
Copy link
Owner

I like Bowerfile a lot.

I'd be fine with changing the method names and file names if other DSL users are.
It's a breaking change for old projects. Not a huge one, but could require a minor version change.

@SergeyKishenin
Copy link
Collaborator Author

All right, then I'll try to add commits to this issue.

@SergeyKishenin
Copy link
Collaborator Author

I think, it would be great if we add a Changelog after releasing a new version

@rharriso
Copy link
Owner

Sounds good to me.
On Sep 23, 2013 1:08 AM, "Sergey Kishenin" [email protected] wrote:

I think, it would be great if we add a Changelog after releasing a new
version


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-24900445
.

@SergeyKishenin
Copy link
Collaborator Author

Will you do that or I should?

@rharriso
Copy link
Owner

Can you include it in your pull request?
On Sep 23, 2013 1:18 AM, "Sergey Kishenin" [email protected] wrote:

Will you do that or I should?


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-24900668
.

@SergeyKishenin
Copy link
Collaborator Author

Done
On Sep 23, 2013 12:19 PM, "Ross Harrison" [email protected] wrote:

Can you include it in your pull request?
On Sep 23, 2013 1:18 AM, "Sergey Kishenin" [email protected]
wrote:

Will you do that or I should?


Reply to this email directly or view it on GitHub<
https://github.com/42dev/bower-rails/pull/29#issuecomment-24900668>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/29#issuecomment-24900691
.

rharriso added a commit that referenced this pull request Sep 23, 2013
@rharriso rharriso merged commit 763f70e into rharriso:master Sep 23, 2013
@rharriso
Copy link
Owner

Cool. Thanks for doing that. Sorry It took me a while to respond just now, you know sleep and all.

I'll get this pushed up to ruby gems post haste.

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

Successfully merging this pull request may close these issues.

2 participants