Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 hubot-scripts@2.17.1 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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants