This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason will be displayed to describe this comment to others. Learn more.
Been here, done this but with sinon timers. I don't know if the process is inherently error prone or we just need some better instructions than the commit message at f9ad13c .
The reason will be displayed to describe this comment to others. Learn more.
Aargh! @timmywil is right, but this just doesn't seem workable unless the default grunt task detects the need to invoke bower and does so automatically.
The reason will be displayed to describe this comment to others. Learn more.
What use case would be for that? How you usually update Sizzle? Do you update bower.json then run grunt default, or just update and commit it without executing any command?
The reason will be displayed to describe this comment to others. Learn more.
currently `bower update`
i guess not "currently", since we you use grunt task for it for quite some time now.
So basically, now you replace bower update with grunt bower plus commit newly updated dependency, i guess detect changes of bower.json file would be easy enough to do but in that case you should have to remember to make commit after grunt default command.
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't update it, see
bowercopytask.cc @timmywil
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Been here, done this but with sinon timers. I don't know if the process is inherently error prone or we just need some better instructions than the commit message at f9ad13c .
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timmywil well, i think "i told you so" would be in order :-)
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It must have seemed like a good idea at the time😸 Worse things happened to me with submodules but I've blocked them out.
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked with @gibson042 over IRC when the workflow changed. I think he just had a brain fart.
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markelog I reciprocate your "I told you so" with a "We commit dependencies now. Deal with it."
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about putting the commit message from f9ad13c into a comment before the
boweralias definition in Gruntfile?9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Should we put it in the README as well?
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but README is not only for commiters and other developers don't really have to care about this issue.
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, that's pretty much why I haven't put it somewhere. Ok, in the Gruntfile seems like a good place.
9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aargh! @timmywil is right, but this just doesn't seem workable unless the default grunt task detects the need to invoke
bowerand does so automatically.9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What use case would be for that? How you usually update Sizzle? Do you update
bower.jsonthen rungrunt default, or just update and commit it without executing any command?9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bower update;grunt)9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically, now you replace
bower updatewithgrunt bowerplus commit newly updated dependency, i guess detect changes ofbower.jsonfile would be easy enough to do but in that case you should have to remember to make commit aftergrunt defaultcommand.9ac88deThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The universe always promises a bigger idiot. Today, that role is mine.