Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor options validation #368

Open
fgreinacher opened this issue Apr 18, 2022 · 2 comments
Open

Refactor options validation #368

fgreinacher opened this issue Apr 18, 2022 · 2 comments
Assignees
Labels

Comments

@fgreinacher
Copy link
Contributor

@travi @JonasSchubert I'd suggest to tackle this in two steps:

  1. Merge this fix to unblock folks asap.
  2. Create a follow-up issue to refactor the option validation. IMO it's super brittle and we should clean it up properly so that we cannot run into these kinds of issue in the future. We should somehow encapsulate options, their validation and error messages better.

Originally posted by @fgreinacher in #364 (comment)

@fgreinacher fgreinacher changed the title @travi @JonasSchubert I'd suggest to tackle this in two steps: Refactor options validation Apr 18, 2022
@fgreinacher fgreinacher self-assigned this Apr 18, 2022
@JonasSchubert
Copy link
Contributor

How shall we continue here?
To keep it similar to semantic-release/github I simply would add the missing validations and write a Unit Test checking whether all validations are covered (Including new validations).
Or did you consider a bigger refactoring @fgreinacher ?

@fgreinacher
Copy link
Contributor Author

fgreinacher commented Apr 28, 2022

I had a slightly bigger change in mind, encapsulating configuration options, validation logic and error messages, conceptually something like this:

options: [
   { 
      name: "assets",
      validate: (value, errors) => {
         if (!value) errors.push("Missing value for option assets...");
         // ...
      }
]

We could afterwards do the same refactoring for the GitHub plugin. WDYT @travi?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants