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.

Sign up for GitHub

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

Jump to bottom

Change type legacy_binary_affirmation_string to legacy_yes_or_no_string #118

Open
jouvin opened this issue Jul 18, 2016 · 3 comments
Open

Change type legacy_binary_affirmation_string to legacy_yes_or_no_string #118

jouvin opened this issue Jul 18, 2016 · 3 comments

Comments

@jouvin
Copy link
Contributor

jouvin commented Jul 18, 2016 •
edited
Loading

As stated in PR quattor/configuration-modules-core#816 ( https://github.com/quattor/configuration-modules-core/pull/816/files#r71087664), legacy_binary_affirmation_string does not make obvious what are the valid values of this type. Renaming this type to legacy_yes_or_no_string may be better and avoid having to look at the type definition to know the valid values.

Making the change involves modifying the configuration modules (core only?) using it. See quattor/configuration-modules-core#749.
@jouvin jouvin mentioned this issue Jul 18, 2016
Merged
@jrha
Copy link
Member

jrha commented Jul 18, 2016

It's a bit late to comment on this, the PR that introduced it was merged in April.
Instead of renaming it I feel like it would be better for us to make the effort to switch the components to use booleans.

@jrha jrha changed the title Change type egacy_binary_affirmation_string to legacy_yes_or_no_string Change type legacy_binary_affirmation_string to legacy_yes_or_no_string Jul 18, 2016
@jrha
Copy link
Member

jrha commented Jul 18, 2016

See #111 and quattor/configuration-modules-core#749 for what would need to be changed.

@ned21
Copy link
Contributor

ned21 commented Jul 18, 2016

Sorry I was on holiday in April so didn't see it to comment! :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ned21 @jrha @jouvin