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

remove support for context.isFunction #438

Closed
wants to merge 1 commit into from

Conversation

prashn64
Copy link
Contributor

No description provided.

@@ -378,7 +378,6 @@
return dust.log(err, ERROR);
}
};
fn.isFunction = true;
return fn;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to define fn variable? Could we do:

return function() {
    ...
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this function can be simplified down to that now.

@kate2753
Copy link
Contributor

Looks good to me. How are we documenting or announcing this deprecation?

@prashn64
Copy link
Contributor Author

That's actually up for discussion. I'm against polluting the console.log window, unless you can get rid of the message by fixing your code. There's no way to do that in this situation.

One place where it will be documented is the Changelog.MD. That might be good enough, but we can also just have a section in the README. I think this would be pretty useful to update as you release a new version, since then you can go back to older tags and see exactly what is/was being deprecated/removed within that release.

Let me know what you guys think about that.

@rragan
Copy link
Contributor

rragan commented Mar 27, 2014

Having a deprecated/removed section by release in the README seems like a good backup to the Changelog.MD. Not everyone looks at the Changelog so repeating ourselves regarding things going away seems worth the small extra effort. Putting in deprecated in release n makes it easy to copy the line to "removed" in release n+1.

@prashn64
Copy link
Contributor Author

Updated to not include the merge commit: #447

@prashn64 prashn64 closed this Mar 27, 2014
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