search

rharriso / bower-rails

Bundler-like DSL + rake tasks for Bower on Rails
MIT License
1.46k stars 128 forks source link

Add check pending feature #151

Open sakirtemel opened 9 years ago

sakirtemel commented 9 years ago

Add check pending feature. If the Bowerfile's modification time is newer than bower.json file then it means that bower.json needs to be updated via using rake bower:install. Raises an exception after rails initialization if the flag is set to true.

It's especially useful when a team is working on the same project and Bowerfile is changing frequently.

ActiveRecord's check pending migrations mechanism has been taken as reference.

SergeyKishenin commented 9 years ago

This is a great feature, thanks! But i'm not sure we should check bower.json this way — https://github.com/rharriso/bower-rails/pull/151/files#diff-eaaf5f02e1f215447a04f9c0176a9f7cR188 — as we gonna have dynamic bower_components directory, see #146.

Alternatively, can we implement a lockfile principle (Bowerfile.lock) somehow here?

sakirtemel commented 9 years ago

I was also not sure about finding bower.json file but as far as I see it's hardcoded. Even there will be dynamic bower_components directory bower.json would have stayed at the same directory. If you can suggest an idea to find it properly it would be appreciated.

And since lockfiles are also added to repositories it won't work for our needs.

SergeyKishenin commented 9 years ago

I was also not sure about finding bower.json file but as far as I see it's hardcoded. Even there will be dynamic bower_components directory bower.json would have stayed at the same directory. If you can suggest an idea to find it properly it would be appreciated.

Yes, I think bower.json location will be permanent if we have the only directory that will store bower components. But firstly we need to get rid of groups feature as I don't see it valuable.

And since lockfiles are also added to repositories it won't work for our needs.

We can use Bundler-like lockfile implementation by checking components versions. Right?

sakirtemel commented 9 years ago

What is groups feature?

We can have bundler-like lockfile, but is it related with this feature?

SergeyKishenin commented 9 years ago

Now you can specify a group by calling

group :lib do
 asset 'backbone'
end

The reason, you submitted this PR is that all the time bower.json should be up-to-date? This also could be achieved by implementing lockfile that seems to be a better solution. Or maybe I'm missing something?

sakirtemel commented 9 years ago

bower.json always needs to be newer than the Bowerfile. If we will have a Bowerfile.lock it would be added to the repository, too.

Let's say developer A added something to the Bowerfile and run rake bower:install when he/she commits adds the Bowerfile.lock as well. Developer B pulls the changes, and there won't be any way to understand that Bowerfile has been changed at developer B's environment. Because the requirements would fit : Bowerfile.lock's modification date would be newer than Bowerfile, as it's commited like that.

That's why I'm checking bower.json, it's also like a lockfile and not commited one.

sakirtemel commented 9 years ago

But maybe we can have an additional lockfile next to the bower.json, and in that file we can store the versions and the checksum of the bowerfile, so instead of checking the mtime we would compute the Bowerfile's checksum every time.

SergeyKishenin commented 9 years ago

That's why I'm checking bower.json, it's also like a lockfile and not commited one.

What if someone commits bower.json to the repository?

But maybe we can have an additional lockfile next to the bower.json, and in that file we can store the versions and the checksum of the bowerfile, so instead of checking the mtime we would compute the Bowerfile's checksum every time.

Sounds better! But how do we calculate checksum after pulling commits? Iterating through installed components?

sakirtemel commented 9 years ago

Hmm, we're ignoring vendor/assets.

We will do it each time before rails runs. But still we need at least one file to be git ignored. Or we can use bower command additionally to be sure. I have to think about it. If you have any other comments would be appreciated.