citation-style-language
Submission bot wishlist
CC @rmzelle @inukshuk
Follow up of https://github.com/citation-style-language/styles/issues/1088
- Allow customized, automated message on travis return
- Include instructions on how to update pull request specific for each pull request (i.e. with link to correct branch in fork)
- Allow customization of message depending on Travis errors: I'm thinking we'll create human-written instructions for the most common travis errors
- Have a separate warning category if possible, where we could point to common issues which don't cause a travis fail, such as missing documentation or default locale.
Here's how I'm roughly envisioning a sample message:
Thank you for submitting to the Citation Styles Language repository. Our automated test suite has found some errors in your submission. Please make sure that your filename, id, and link follows the conventions outlined in 1,2,4,5,6 here.
While not technically required, we also ask that you include a link to online documentation for the style with
rel="documentation"as per 10. here.You can update your pull request by editing your style [here --include link to correct branch]. Click the pencil icon at the top right of that page.
Please let us know when you're done with all edits, as we don't get notified for updated pull requests. Feel free to post here if you need any help.
Good start. Some additional thoughts:
- maybe we can reuse the https://github.com/csl-bot account to post the reports?
- it would be good if there was a special warning if the Travis CI build failed because of other reasons. Sometimes a build just doesn't complete, and it would be nice to let the submitter know it's not their fault. E.g:
Thank you for your submission. Unfortunately, our automated Travis CI test suite failed for unknown reasons. Please wait until the CSL team restarts the build.
- pull requests can cover multiple files, so it would be good to give a per-file list of errors. It would be good to have cut-off based on e.g. the number of files, or the number of lines in the report, in case somebody really screws up a commit. E.g. (based on https://travis-ci.org/citation-style-language/styles/builds/55675997):
Thank you for your submission. We review each submission by hand, but our automated Travis CI test suite already found some issues:
- padagogische-hochschule-vorarlberg
- is not a valid CSL style. Please see https://github.com/citation-style-language/styles/wiki/Validation and make sure your style validates
- calls an undefined macro: "year-date"
You can update this pull request by visiting the style link(s) above and clicking the pencil icon.
- when new commits trigger additional bot reports in a given pull request, it would be nice if these subsequent reports are more concise. E.g. text like "Thank you for your submission" doesn't need to be repeated over and over again.
- The "Please let us know when you're done with all edits" should not be necessary if we have the bot, since we could have it always post, on fail and success.
Also, I imagine that there is a wider interest in a tool like this that can parse Travis CI/RSpec reports and post back to GitHub, so if you can make the bot more modular/configurable without too much extra effort, that might be good to keep in mind as well.
I would also be nice to provide a link to the validator in case of invalid styles. E.g. for https://github.com/peboeck/styles/blob/patch-8/padagogische-hochschule-vorarlberg.csl, provide the link http://validator.citationstyles.org/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fpeboeck%2Fstyles%2Fpatch-8%2Fpadagogische-hochschule-vorarlberg.csl&version=1.0.1
Yes, all good additions/changes, no objections. Very much agree on trying to make this easily adaptable for other people.
Do you have any preference regarding the implementation language? I'm leaning towards Ruby just in case it will be necessary to run any of the tests locally.
Do you think it would make sense to use two hooks: one for GitHub Pull Requests and one for Travis CI builds. This way, we could respond to each Pull Request with a general comment along the lines of the example @adam3smith posted above; whilst the response to each Travis build could focus solely on the results of the build (without the need to distinguish between the initial Pull Request and later commits). Do you think that would make sense?
It would also provide immediate feedback upon creation of a pull request, which is a good thing.
@rmzelle have you decided if you'd like to reuse @csl-bot to post Sheldon's comments? (I'm all for it) Can you create an access token for me to use to post comments as @csl-bot for testing? (We just need a secure channel for you to send it to me... perhaps a GPG encrypted mail?)
@dstillman, can you help @inukshuk out with access to the @csl-bot account?
We could also create a separate account, like "sheldon-bot" or "Shelbot" (the latter seems to be the official name for Sheldon's robot: http://bigbangtheory.wikia.com/wiki/The_Cruciferous_Vegetable_Amplification ; both usernames are available on GitHub), which would allow us to give the bot a more human face.
Would two accounts be preferable, security-wise?
Humble beginnings: https://github.com/inukshuk/styles/pull/3
Will hopefully have a first version for you to experiment with in a couple of days!
Will hopefully have a first version for you to experiment with in a couple of days!
Is there anything I should wait for, or can I start submitting PRs for your repo already (e.g. to change the text)?
Let's wait until I have the build hooks ready -- I'll be much quicker to add/adjust features then.
I added headers to the various submission criteria at https://github.com/citation-style-language/styles/wiki/Style-Requirements, so they now have HTML anchors we can link to. E.g. https://github.com/citation-style-language/styles/wiki/Style-Requirements#1---title-abbreviations
I've added build passed/failed templates now, so I think we can start thinking about the actual content of the templates and how to roll this out. Currently the bot is hosted on Heroku and is automatically updated if you push any changes and the tests pass on Travis CI. I think this is a pretty good setup, because this will allow us to make changes to a template and just do a git push to deploy it.
Since we want to enable messages on failed builds I will make a PR to the distribution-updater bot to ignore failed builds, OK?
The payload sent by Travis does not give us any details on why a build failed; in the long run we could consider having our bot checkout the commit and do its own testing, but it probably makes more sense to simply improve the output of the test suite.
Here is an example with failing and passing builds: https://github.com/inukshuk/styles/pull/4
Since we want to enable messages on failed builds I will make a PR to the distribution-updater bot to ignore failed builds, OK?
I'm confused. csl-bot already only pushes to the "styles-distribution" if the Travis build of the "styles" repo succeeds. What needs to be changed?
The payload sent by Travis does not give us any details on why a build failed
It would be nice if we could steer users to the relevant sections of https://github.com/citation-style-language/styles/wiki/Style-Requirements, though. I found https://github.com/travis-ci/travis-ci/issues/239, which shows that structured test metadata is unfortunately not to be expected anytime soon. The travis.rb library allows for easy access to the build log (https://github.com/travis-ci/travis.rb#logs), although I don't know if that's any easier than just checking the log online. We can probably use some regex to identify which tests failed and format the GitHub feedback comment appropriately, no?
I'm confused. csl-bot already only pushes to the "styles-distribution" if the Travis build of the "styles" repo succeeds. What needs to be changed?
Well, that's it: if we want to receive notifications on failed builds in the future, we will need to change the Travis configuration. Once we do that, the styles-distribution hook will be called for failed builds as well, so we should make sure those will be ignored before we change the configuration.
I don't know if it is possible to have different settings per hook; at least that is my interpretation of the docs at: http://docs.travis-ci.com/user/notifications/#Webhook-notification
I've only glanced at the distribution-updater, but do you currently ignore builds triggered by PRs? If not, that's something we should probably do as well, isn't it?
Regarding the test suite, we should definitely make the output of failed test easier to understand; this will be helpful in and of itself and is also necessary if we want to be able to use the output to generate more helpful comments (so we should do that either way).
I suggest that we finish up a first iteration of the submission bot first (other than the templates itself we can still experiment with commit-based comments), and then work on improving the test suite output and adjusting the bot's comments. Does that sound good?
I don't know if it is possible to have different settings per hook
Ah, okay, it looks like it's not.
I suggest that we finish up a first iteration of the submission bot first
Sure. You just need text for https://github.com/inukshuk/Sheldon/tree/master/templates at this point?
Exactly. Depending on the information we want to be able to display, we may have to expose more locale variables to the template. Currently, the pull request template is passed the payload of GitHub's PR event and the two build events are passed the payload of the Travis notification.
Would it be possible and/or reasonable to automatically test, whether the url(s) for documentation are still alive?
@zuphilip, while good to check, that something we should ideally do for all the styles in the repository, not just for new submissions.