Pull Request review template. Feel free to edit (this is a wiki)
- Does it fix the problem described in the issue?
- Not fixing additional problems not described in the issue.
- No use of .to_sym, .send or other reflection on untrusted inputs.
- Not missing string extractions (use :mark_translated: true).
- All string extractions follows our rules.
- Added appropriate permissions for non-admin users?
- New code hasn’t been copy-pasted from elsewhere.
- Do new fields have appropriate validators?
-
Do validators use Rails 3 syntax? (
validates <field>, <conditions>) - No exceptions are squashed.
- Are raised exceptions type of Foreman::Exception?
- Unit tests present?
- Functional tests present?
- New view templates have .html.erb extensions, not just .erb
-
New view templates use
%>instead of-%> - Mixins use ActiveSupport::Concern fully, with no class_eval, InstanceMethods etc.
- Concerns and new classes are added to app/ and not lib/
- All new APIs, or additional model attributes have apipie documentation and are in API views
- Apipie documentation is correct: HTTP methods, names of parameters, required true/false
- Commit message follows the [[Reviewing_patches-commit_message_format]]
- scoped_search definitions are added for new model attributes where appropriate
- scoped_search definitions for virtual fields (:ext_method) use :only_explicit
- API functional tests have a valid test first, followed by invalid tests
- Pass block to logger.debug to lazily evaluate expensive debug statements
- New Javascript files are added to config/environments/production.rb if not in app.js
- No new “stylesheet” tags are added to views, they’re already in app.css
- Deprecations use Foreman::Deprecation with a deadline of latest stable + three
- Update license file when vendoring 3rd party code/assets etc
