resolves #658

[armish]

More information here: #658

[ihodes]

Maybe stick this between line 68 and 68 with a note that it only needs to be run when changing querylanguage.pegjs?

[danvk]

I typically just run gulp prod in lieu of more specific subcommands. Any reason not to just run that?

[ihodes]

It takes longer than just running gulp peg and the extra work is not needed. Any reason to tell users to use gulp prod instead? We should also put the peg task into the default task, obviating the need for this entirely, but adding more startup time each time you start gulp.

[danvk]

Does it actually take substantially longer? IIRC gulp build is the rate-limiting step here. My general preference is to just remember one command that always results in a correct state, even if it's a little slower, but to each his/her own.

[ihodes]

My general preference is to just remember one command that always results in a correct state

That's what I'm suggesting with sticking peg into the default task; we're on the same page here :)

@armish Sorry that such a small change is getting outsized discussion, but I agree that sticking the peg task into the default task list is the best way to handle this. Then we don't need to mention it in the README/DEVELOP.md at all.

[armish]

No worries -- I find these discussion really helpful. I also think it makes
sense to to add peg into the default tag. I will update the PR
accordingly.

On Tue, Jun 9, 2015 at 1:13 PM Isaac Hodes notifications@github.com wrote:

In DEVELOP.md
#659 (comment):

@@ -73,6 +73,7 @@ gulp # Compile the JS and start the automatic
To regenerate the bundled.js file without using the live reloader, run:

+gulp peg

My general preference is to just remember one command that always
results in a correct state

That's what I'm suggesting with sticking peg into the default task; we're
on the same page here :)

@armish https://github.com/armish Sorry that such a small change is
getting outsized discussion, but I agree that sticking the peg task into
the default task list is the best way to handle this. Then we don't need
to mention it in the README/DEVELOP.md at all.

—
Reply to this email directly or view it on GitHub
https://github.com/hammerlab/cycledash/pull/659/files#r32037989.

[ihodes]

Looks good—one comment—and thanks for the PR! :)

[ihodes]

@armish Mind revising this PR in light of the commit (moving the location of the change), and we can merge it in?

[armish]

@ihodes ah, my bad -- sorry about this; I missed your comment and thought this was on your plate. Will revise it ASAP.

[ihodes]

No worries/rush at all. Thanks for taking care of it!

[ihodes]

This is where the niceness of git can fall apart a bit! Instead of merging master into your branch, which can cause all sorts of weird commits (e.g. the commits you pulled in from Jeff and me) and so, sometimes, merge issues, rebasing on master is usually the nicest approach. I think it might be best if you were to 1) squash your commits down to one (undoing some inadvertent changes like adding a space to the DEVELOP.md file) 2) rebasing it on master, if necessary (it shouldn't be; just doing 1) should be enough). Easier still, though "cheating" and maybe not what you want to do, would be to just make a new branch off of master with just this little change in it.

[armish]

Coming from a mercurial background, I was trying to stay away from rebasing, but I guess I have to learn taming the beast sooner than later. Let me try the hard way first and if it fails, I will cheat. Thanks for the hints, @ihodes!

[ihodes]

Thanks for handling this :)

[armish]

it was a learning experience for me (click to see how a single PR changes this guy's life) ;)