Skip to content

Forbid extra fields in engine plugin config types#147

Merged
gpauloski merged 1 commit intomainfrom
issue-124
Sep 6, 2024
Merged

Forbid extra fields in engine plugin config types#147
gpauloski merged 1 commit intomainfrom
issue-124

Conversation

@gpauloski
Copy link
Copy Markdown
Contributor

Description

This is a revert of df1be9f. The justification there to ignore extra fields is overriding the plugin kind in a config file from the CLI args. This is a nice feature, but it comes at the risk of not catching typos in config field names. I think it's ultimately better to be overly strict at the cost of some flexibility over letting simple and subtle errors in a config sneak in.

Fixes

Type of Change

  • Bug (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Internal (refactoring, performance, and testing)
  • Breaking (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (no changes to the code)
  • Development (CI workflows, packages, templates, etc.)
  • Package (package dependencies and versions)

Testing

N/A

Pull Request Checklist

Please confirm the PR meets the following requirements.

  • Relevant tags are added (breaking, bug, documentation, enhancement, package, etc.).
  • Code changes pass pre-commit (e.g., ruff, mypy, etc.).
  • Tests have been added to show the fix is effective or that the new feature works.
  • New and existing unit tests pass locally with the changes.
  • Docs have been updated and reviewed if relevant.

@gpauloski gpauloski added the enhancement New features or improvements to existing functionality label Sep 6, 2024
This is a revert of df1be9f. The
justification there to ignore extra fields is overriding the plugin kind
in a config file from the CLI args. This is a nice feature, but it comes
at the risk of not catching typos in config field names. I think it's
ultimately better to be overly strict at the cost of some flexibility
over letting simple and subtle errors in a config sneak in.
@gpauloski gpauloski merged commit c210f11 into main Sep 6, 2024
@gpauloski gpauloski deleted the issue-124 branch September 6, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New features or improvements to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't ignore extra config options

1 participant