Skip to content

Conversation

@jakearchibald
Copy link
Contributor

No description provided.

@callumacrae
Copy link
Member

Gulp docs say it's a string: https://github.com/gulpjs/gulp/blob/4.0/docs/API.md#gulplastruntaskname-timeresolution

Undertaker docs say it's either a string or a function: https://www.npmjs.com/package/undertaker#lastruntask-timeresolution

Is this patch as a result of something breaking or from reading docs?

(I actually prefer the function syntax. Just wondering)

@jakearchibald
Copy link
Contributor Author

It was failing when I used a string - maybe that's a bug.

@jakearchibald
Copy link
Contributor Author

The error is AssertionError: Only functions can check lastRun when passing a string.

@phated
Copy link
Member

phated commented Oct 11, 2016

I believe it tries to look up the task function by string and checks last-run of the function. That assertion might be due to a failed lookup of the string. I'm on mobile currently and can't dig into the code.

@phated
Copy link
Member

phated commented Oct 11, 2016

Just tested the example and it works. However, because it is a snippet, it doesn't portray that the images function needs to be registered as a task with gulp.task(images). When it isn't registered as a task, I get a thrown error but it isn't the same as the one @jakearchibald encountered.

@jakearchibald would you like to change this PR to include the task registration portion? I think that would make more sense to new users.

@jakearchibald
Copy link
Contributor Author

I don't have strong feelings, but gulp.lastRun(fn) feels easier to understand than gulp.lastRun(string).

With since: gulp.lastRun(images) I read "Since the last run of the images function".

With since: gulp.lastRun("images") I wasn't sure. I figured that the string might just be an ID, so it'd be "Since the last call of gulp.lastRun("images")", but then that didn't work. If I also saw gulp.task(images) I'd be similarly confused, as I'd need to know that somewhere behind the scenes it was getting the name of the function images and storing it.

@phated
Copy link
Member

phated commented Oct 19, 2016

I'm fine with that change too.

@jakearchibald
Copy link
Contributor Author

If you're happy with lastRun(fn), then this PR is good to go, unless I'm missing something.

@callumacrae callumacrae merged commit abbd8ad into gulpjs:4.0 Oct 24, 2016
@callumacrae
Copy link
Member

Thanks! 😃

@callumacrae
Copy link
Member

@jakearchibald would you like to change this PR to include the task registration portion? I think that would make more sense to new users.

I guess that's why it wasn't merged! May have merged this prematurely - my mistake. 😳

I'll add the task registration to the README now.

@jakearchibald
Copy link
Contributor Author

The task registration portion isn't needed if you're providing a function to lastRun.

callumacrae added a commit that referenced this pull request Oct 24, 2016
@callumacrae
Copy link
Member

Right, makes sense now. Whoops!

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.

3 participants