[Rubocop] Enforce entity field type consistency

Description of the proposal

  • Ensures that all Grape API Entity exposed fields have a valid type defined
  • The purpose of linting this attribute is for the purposes of improving out OpenAPI documentation
  • it is helpful for our api definitions to be consistent and type definitions benefit api users

How to Validate these changes

Running the linter

  1. Open any grape entity file that is not listed in .rubocop_todo/api/entity_field_type.yml and remove the type from any documentation hash
  2. Run the new linter on that file ie rubocop --only API/EntityFieldType lib/api/entities/<name of file>
  3. Observe that an offense indicating that the **'API parameter is missing type declaration.'**
  4. Change the file back and update the type to an invalid format - type: :string
  5. Observe a different offense indicating the type definition is invalid

Ensuring files with exceptions will be ignored

  1. run rubocop --only API/EntityFieldType --format files - no files should be listed
  2. Remove one of the files from .rubocop_todo/api/entity_field_type.yml
  3. run run rubocop --only API/EntityFieldType --format files - see the file you removed listed

*** Add the file back to the exception list ***

Check-list

  • Make sure this MR enables a static analysis check rule for new usage but ignores current offenses.
  • Mention this proposal in the relevant Slack channels (e.g. #development, #backend, #frontend).
  • If there is a choice to make between two potential styles, set up an emoji vote in the MR:
    • CHOICE_A: 🅰️
    • CHOICE_B: 🅱️
    • Vote for both choices, so they are visible to others.
  • The MR doesn't have significant objections, and is getting a majority of 👍 vs 👎 (remember that we don't need to reach a consensus).
  • (If applicable) One style is getting a majority of vote (compared to the other choice).
  • (If applicable) Update the MR with the chosen style.
  • Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK
  • Follow the review process as usual.
  • Once approved and merged by a maintainer, mention it again:
    • In the relevant Slack channels (e.g. #development, #backend, #frontend).
    • (Optional depending on the impact of the change) In the Engineering Week in Review.

#574002 (closed)

Edited by Vlad Wolanyk

Merge request reports

Loading