If we see a BZ URL in one commit, but not the others, or in the PR body itself, but not in the commits, warn the PR author that they may need to fix their commits.

At a minimum just allow the common words from GitHub like "Fixes", "Resolves" in front of a BZ URL.

Eventually "Fixes" could actually mean moving the BZ to POST automatically.

If the commit diff matches the regex \\[xu]?\d\d?\d?\d?, the bot should remind you that the default ruby script encoding in 2.0+ is UTF-8 and to use US-ASCII if they are binary string literals and not actual UTF-8 characters.

See:
ManageIQ/manageiq#804

Can be done simply at first by checking the PR title .start_with?("WIP") || .start_with?("[WIP]")

Use case: I am renaming a large old file (shows up as an add and delete in git) without changing any of the content. Rubocop comments on the PR with 400+ issues in the old file.

It would be nice to be able to add [no complaints] to the commit message and have rubocop ignore those changes in its comment.

Give a grace period of 10 minutes or so, so as not to attack to quickly.

The Github API only shows members who publicize their membership, so when the issue manager filters commands by user, it doesn't know about private users. See https://developer.github.com/v3/orgs/members/#members-list

At first, it can link to a gist created by running bin/rails r "" and that would enable us to see any Gems that are required by bundler and pull in many other libraries or consume lots of memory.

Later, we can support Gemfile entries that say :require => false to see how much time/memory the require will cost.

See: ManageIQ/manageiq#1170

And vice versa.

We should detect migrations mentioning these statements without a null value and warn that the default is changing from true to false in rails 5.

From ManageIQ/manageiq#3145

Use existing timestamp behavior of allowing nulls (t.timestamp, t.timestamps)

Fixes warning:
`#timestamp` was called without specifying an option for `null`.
In Rails 5, this behavior will change to `null: false`.
You should manually specify `null: true` to prevent the behavior of your existing migrations from changing.

When a PR is submitted without a description, the bot should remind people to add a description that describes why the PR is important to the ManageIQ codebase.

Possibly even include some rules around not just describing the changes, but describing the purpose of the changes.

Change CyclomaticComplexity, MethodLength, ClassLength, LineLength, WordArray to be recommendations

DoubleNegation.

It should honor: @miq-bot reload_labels and @miq-bot reload_milestones, or @miq-bot reload_repo if you're in the organization.

We cache these values at startup and we have to restart the bot to get new labels or milestones.

Existing code checks for these labels before allowing the user to assign that label

• Certain cops are being bucketed incorrectly. For example, "Line length too long" is coming up as a warning instead of a lower level severity. See ManageIQ/manageiq#3126 (comment) for an example
• On that same example, it shows line length max as 80, when the it should be 120. This implies it's not honoring the .haml-lint file properly.

@skateman Can you figure these out?

It would be nice if the bot pinged the author of this PR to ensure they clarify why tests aren't included in the PR. Many times the reviewer needs to ask this and either forgets or feels like the bad guy. The bot should do this automatically to keep humans out of it.

It could be as simple as checking for any changes in test or spec directories.

“Please clarify why this PR doesn't include tests, whether existing tests cover the code in the PR or some other reason.”

If I update a branch like botvinnik, then the current master branch build gets killed. This is because the code is lumping all non-PRs together, and probably should group them by branch/tag name instead.

For example prefer .to_sym over .intern.

The existing preferred methods cop only works on collections.

Problem: Sometimes Travis build error (for timeouts or other reasons than code failures). I don't have permissions to restart because I'm not a merger on manageiq. So I need to either close and re-open the PR to re-run everything or ping someone to re-run the component that errored.

Possible solutions:

• Allow a PR comment like "@miq-bot travis_restart vmdb"
• Let the bot automatically restart errored builds (not sure if there are potential loop conditions)

It'd be great when the bot could parse also the *.haml files.
By forcing the haml-lint gem to use rubocop and passing the original .rubocop.yml config file it's quite easy.

What do you think? #30

As a reviewer, I would like to be able to find all PRs that are in the Providers space easily. The only way I know how to do this is to query for all PRs that have a providers label. This relies on someone setting the providers label on provider-specific PRs.

I would like it better if the bot simply nagged the PR author to label the PR.

Scan for all PRs older than 1 day and that do not have a component (blue) label. For each PR in this category, send an email to the commiters list and to the PR author.

The Email should contain the link to the PR as well as some short instructions on how to apply a label if the PR author does not have access to label PRs.

Component (or, blue) labels include:

• api
• appliance
• fleecing
• providers

... and may contain slashes:

• providers/containers
• appliance/console

See https://github.com/ManageIQ/miq_bot/blob/master/app/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder.rb#L48

@skateman Can you do this one?

I have a design for this, which I've shared with @jrafanie. I'll have to elaborate here or I'll just make a PR.

Check for any new migrations that create_table or rename_table as there may be side effects due to replication. In a PR, let the author know via comment.

If not already set, set the milestone. Give a grace period of a few minutes or so, possibly, to not step on a merger's actions.

The bot should complain in a PR if the diff contents contain certain unwanted strings like "cfme".

It seems that the documentation links are broken because rubocop changed the casing of the module.

http://www.rubydoc.info/gems/rubocop/frames/Rubocop/Cop/Style/CaseIndentation
vs
http://www.rubydoc.info/gems/rubocop/frames/RuboCop/Cop/Style/CaseIndentation

We can should probably

• Update to the new format
• Update rubocop as well
• Change the links to have the version number in them? e.g. http://www.rubydoc.info/gems/rubocop/0.21.0/frames/Rubocop/Cop/Style/CaseIndentation
• Take the "frames" out of the link. e.g. http://www.rubydoc.info/gems/rubocop/0.21.0/Rubocop/Cop/Style/CaseIndentation

If you add_label WIP when the label is "wip", it fails.

It should mark these comments as private

Use case:
A reviewer will look at a PR and possibly give it a 👍 , but want to wait for the tests to pass. Said reviewer forgets to come back to it. Openshift bot does something similar where you can add a comment with [merge] and it will run the tests and merge if it passes. Additionally, it would be convenient to be notified when the tests fail.

For example:
openshift/origin#448 (failed -> doesn't merge and comments)
openshift/origin#409 (passed -> merged and comments)

Check the specs directory (except spec/support/* and spec_helper.rb) for new files that don't end in _spec.rb

If a commit message has a bugzilla url, it should be labeled bug. if that commit is removed, it should be unlabeled.

For newcomers, we should link to the current yml and the how to setup rubocop locally doc.

For oldies, it would be useful to easily grab the new yml if there are cop messages online but not locally (user has an old yml)

For maintainers, it would be useful to link directly to the yml the bot is using,at a commit, to verify they updated the bot to the latest code.

Summary:

• Current yml so users can shift click and download
• Link to current bot commit or tag (run using bot at commit xyz)
• link to setting up rubocop locally

Fix when git diff has invalid utf-8 byte sequences:
Example on cfme:
git diff --patience -U0 --no-color 35f076630075fca14ecf6b12589fe8254f4911b1\~..a74df1236223d4d0726dca368c244bcb7a184ff9

When people triage them, they can remove this label. This should help to keep track of issues.

The alternative is to have a way to filter all issues without the normal set of component labels "appliance" or "UI" , etc.

Many merge commits have information, but generally that information is from the commits themselves. This causes duplicate messages in Bugzilla which are mostly noise.