Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

Fixes #20307 - Add unicode char check to certs check #521

Merged
merged 1 commit into from
Jul 24, 2017
Merged

Fixes #20307 - Add unicode char check to certs check #521

merged 1 commit into from
Jul 24, 2017

Conversation

chris1984
Copy link
Member

Fixed a few grammer mistakes as well on some of the other functions.

@theforeman-bot
Copy link

Issues: #20307

@chris1984
Copy link
Member Author

Example of output:

Checking for non unicode characters[FAIL]
There are non unicode characters present:

/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:1426:# Certinomis - Autorité Racine /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:3058:# NetLock Arany (Class Gold) Főtanúsítvány /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:4223:# TÜRKTRUST Elektronik Sertifika Hizmet Sağlayıcısı /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:4395:# TÜBİTAK UEKAE Kök Sertifika Hizmet Sağlayıcısı - Sürüm 3 /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:4427:# TÜRKTRUST Elektronik Sertifika Hizmet Sağlayıcısı H5 /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem:4454:# TÜRKTRUST Elektronik Sertifika Hizmet Sağlayıcısı H6

@chris1984
Copy link
Member Author

@ekohl can I get a review please 🍻 build failure is unrelated.

@ekohl
Copy link
Member

ekohl commented Jul 18, 2017

theforeman/puppet-katello#198 and theforeman/puppet-foreman_proxy_content#129 should fix the build issue.

@@ -112,11 +112,24 @@ function check-ca-bundle () {
fi
}

function check-non-unicode () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean non-ascii instead of non-unicode?

@chris1984
Copy link
Member Author

[test]

@chris1984
Copy link
Member Author

@ekohl can I get another review please, also if it looks good can you kick off the travis job again I dont have the ability to.

@ekohl
Copy link
Member

ekohl commented Jul 21, 2017

I'm not sure why it's still not able to do a build, but that's not related to this PR.

@chris1984
Copy link
Member Author

@ekohl cool thanks for the review 🍺 since its not related did you want want for it to get fixed before merge?

@chris1984
Copy link
Member Author

@ekohl since the tests are not related did you want to merge or hold off and wait?

@ekohl
Copy link
Member

ekohl commented Jul 22, 2017

I was hoping for a second ACK but I don't mind merging.

@ekohl ekohl merged commit 4cdf033 into Katello:master Jul 24, 2017
ehelms pushed a commit to ehelms/katello-installer that referenced this pull request Jul 31, 2017
ehelms pushed a commit that referenced this pull request Jul 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants