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

Print warning for loading deprecated hubot-scripts.json #970

Merged
merged 6 commits into from
May 6, 2016

Conversation

bkeepers
Copy link
Contributor

@bkeepers bkeepers commented Jun 8, 2015

Instead of going through and manually adding deprecations for individual scripts (/cc github/hubot-scripts#1641), what if core prints a warning whenever it loads the deprecated scripts?

This will print the following warning after successfully loading scripts from `hubot-scripts.json:

Loading scripts from hubot-scripts.json is deprecated and will be removed in 3.0
(https://github.com/github/hubot-scripts/issues/1113). See https://hubot.github.com/docs/#scripts
to find a replacement for these scripts: [ 'alot' ]

@@ -111,6 +111,11 @@ else
console.error "Error parsing JSON data from hubot-scripts.json: #{err}"
process.exit(1)

console.warn "Loading scripts from hubot-scripts.json is deprecated and " +
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be inside the above try-catch (scripts could be undefined at this point)

@michaelansel
Copy link
Collaborator

I like this (pending code comments)

@technicalpickles
Copy link
Member

The main downside of this approach is it pushes the work of finding a replacement for a script in the hubot-scripts repository with an npm replacement to every single users. If we can give users specific things they can do as part of an upgrade, I think that'd be much preferred.

Also worth noting, loadExternalScripts has support for a syntax like:

{'hubot-scripts': ['alot'], 'hubot-pager-me': '*'}

You can see the syntax at https://github.com/github/hubot/blob/master/src/robot.coffee#L285-L290 checks for an array or treats it as an object otherwise. The code in the hubot-example and later the generator-hubot puts this in index.coffee:

module.exports = (robot, scripts) ->
  scriptsPath = path.resolve(__dirname, 'src')
  if fs.existsSync scriptsPath
    for script in fs.readdirSync(scriptsPath).sort()
      if scripts? and '*' not in scripts
        robot.loadFile(scriptsPath, script) if script in scripts
      else
        robot.loadFile(scriptsPath, script)

That gets users using the external-scripts.json syntax at least, but doesn't ween them off that repository.

@technicalpickles
Copy link
Member

I'm coming around to this idea. I think the key is going the user a path forward. If we can get hubot-scripts updated to warn about deprecated scripts with specific replacements, then we can still use github/hubot-scripts#1641 as the place to direct people. We could mess with the body a little to make it clear it's not a comprehensive list, but there might be other scripts already in the while, and if there aren't users should be encouraged to take ownership of them.

@cycomachead
Copy link
Contributor

then we can still use github/hubot-scripts#1641 as the place to direct people. We could mess with the body a little to make it clear it's not a comprehensive list, but there might be other scripts already in the while, and if there aren't users should be encouraged to take ownership of them.

Just thought about this -- what about a Wiki page on the hubot-scripts repo?
(Then everyone could add to it in an easily searchable manner.)

@technicalpickles
Copy link
Member

Having it in a wiki would make it easier for anyone to update. The downside is that anyone can update, which means it'd need to be curated for accuracy.

@technicalpickles
Copy link
Member

Been noodling about this one some more. I was thinking to add a replacements.json to hubot-scripts, that hubot can check for replacements. Here's what I have for output so far:

[Sun Jan 10 2016 19:27:00 GMT-0500 (EST)] WARNING Loading scripts from hubot-scripts.json is deprecated and will be removed in 3.0 (https://github.com/github/hubot-scripts/issues/1113), in favor of packages for each script.
Here is a list of known replacements for scripts you have enabled. Follow the link for installation instructions, then remove from hubot-scripts.json:
        9gag.coffee for https://github.com/luijose/hubot-9gag
        asana.coffee for https://github.com/hubot-scripts/hubot-asana
These scripts you are using don't have (known) replacements. You can copy the script into your local hubot, or consider helping maintain it yourself:
        corgime.coffee

I'm not quite happy on the language/wording yet, but I'm aiming to:

  • tell user to delete hubot-scripts.json if it's empty
  • link to scripts that have replacements, and tell to follow those instructions and remove the item in hubot-scripts.json
  • for scripts without replacements, copy the script into your local hubot and/or consider helping maintain it

@technicalpickles
Copy link
Member

github/hubot-scripts#1697 is shipped in [email protected] to start tracking that replacements.json, so going to see about landing this to start getting the word out.

@technicalpickles technicalpickles merged commit 5770906 into master May 6, 2016
@technicalpickles technicalpickles deleted the warn-hubot-scripts branch May 6, 2016 22:59
@technicalpickles
Copy link
Member

This is released in 2.19.0

@bkeepers
Copy link
Contributor Author

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.

5 participants