X Tutup
The Wayback Machine - https://web.archive.org/web/20201123153104/https://github.com/shelljs/shelljs/pull/458
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

chore: revert depreciate shelljs/make (#431) #458

Merged
merged 1 commit into from Jun 13, 2016
Merged

Conversation

@zephraph
Copy link
Contributor

@zephraph zephraph commented Jun 9, 2016

This reverts commit 5a31c7c from #431.

This reverts the deprecation of the make command as noted in #340.

This reverts commit 5a31c7c.
@zephraph zephraph changed the title Revert "chore(make): depreciate shelljs/make (#431)" Revert depreciate shelljs/make (#431) Jun 9, 2016
@nfischer nfischer changed the title Revert depreciate shelljs/make (#431) chore: revert depreciate shelljs/make (#431) Jun 9, 2016
@nfischer
Copy link
Member

@nfischer nfischer commented Jun 9, 2016

I'll leave this to @ariporad to review. This commit seems to properly revert the feature, if that's something we want to do.

Alternative approaches would be to display a different warning message instead.

@zephraph
Copy link
Contributor Author

@zephraph zephraph commented Jun 10, 2016

What different message would be displayed? Unless the deprecation is followed through with I wouldn't think there's a need to have a warning message.

@ariporad
Copy link
Contributor

@ariporad ariporad commented Jun 10, 2016

@zephraph: we've decided that this till will be unsupported going forward, so we're going to say something along those lines. Do you want to go ahead and add that, or should I?

@zephraph
Copy link
Contributor Author

@zephraph zephraph commented Jun 10, 2016

I don't think a message is necessary. Deprioritize it as a feature for support and shift the burden of maintenance onto the community. We could add a message in the documentation to explain as much.

@nfischer
Copy link
Member

@nfischer nfischer commented Jun 10, 2016

@zephraph's approach sounds good to me. LGTM

@ariporad
Copy link
Contributor

@ariporad ariporad commented Jun 10, 2016

SGTM

@nfischer nfischer merged commit 353138c into shelljs:master Jun 13, 2016
3 checks passed
3 checks passed
approvals/lgtm this commit looks good
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.
X Tutup