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

Rename "country" property to "coutryCode" #6

Open
rendertom opened this issue Jan 6, 2019 · 6 comments
Open

Rename "country" property to "coutryCode" #6

rendertom opened this issue Jan 6, 2019 · 6 comments

Comments

@rendertom
Copy link

It is not clear what "country" property under selfSignedCert operation expects. Name suggests it has to be a full name country (Lithuania, Estonia), however, it expects a 2 letter coutryCode. So to avoid further confusion it would be beneficial to rename this property to countryCode instead.

@codearoni
Copy link
Owner

After looking at the zxp sign cmd docs, they also label it as countryCode.

I'll consider this, but it'll be a breaking change. This will come with a zxp-sign-cmd 2.x.x release

@GitBruno
Copy link

GitBruno commented Jan 10, 2019

I don't think there is a need to update the variable name. countryCode is just as misleading as there are numeric and alpha-3 codes. But as pointed out the alpha-2 is needed. Said that, it would be good if this was more clearly communicated.

Firstly we can update the readme:

From:

Property Required Datatype Purpose
country yes String The country associated with the certificate

To:

Property Required Datatype Purpose
country yes String The country code associated with the certificate (ISO 3166-1 alpha-2 code)

And secondly, we should check the input on validity and display a nice error message when the user makes a mistake.

@codearoni
Copy link
Owner

Doc changes would help. I'm not comfortable doing validation in node.js-land.
If the zxp binary accepts it, then so be it. This module is a wrapper.

@GitBruno
Copy link

Validation can be as simple as /^[a-zA-Z]{2}$/. Said that, error reporting is more important than validation.

@rendertom
Copy link
Author

+1 on error reporting.

@codearoni
Copy link
Owner

So looking at this in 2020 (time flies), changing the argument would potentially break the ~3 dozen modules using zxp-sign-cmd.

This will be setup as "countryCode" with a fallback to the existing "country" and a console warning for deprecation of the old argument key.

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

No branches or pull requests

3 participants