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

Jump to bottom

client, platform: Use paths.SSH* instead of get_config_dir(). #294

Closed
wants to merge 1 commit into from
Closed

client, platform: Use paths.SSH* instead of get_config_dir(). #294

wants to merge 1 commit into from

Conversation

tjaalton
Copy link
Contributor

@tjaalton tjaalton commented Dec 1, 2016 •
edited by MartinBasti

This cleans things up a bit.

@tjaalton tjaalton force-pushed the master branch 2 times, most recently from d18baf9 to ec8abf4 Compare December 1, 2016 12:28
@tjaalton
Copy link
Contributor Author

tjaalton commented Dec 1, 2016

I don't understand why travis claims it failed while the build passed fine

@mirielka
Copy link
Contributor

mirielka commented Dec 1, 2016

Build passes fine, pep8 does not:
./ipaclient/install/client.py:1006:80: E501 line too long (82 > 79 characters)
./ipaclient/install/client.py:1029:80: E501 line too long (83 > 79 characters)

@tiran
Copy link
Member

tiran commented Dec 1, 2016

@tjaalton Travis CI output is a bit confusing. PEP 8 checks are performed first. The remaining checks are still executed.

@martbab What do you think about improving the output to make it friendlier to outside contributors? Either fail fast or repeat PEP 8 errors at the end of the log.

@tjaalton
Copy link
Contributor Author

tjaalton commented Dec 1, 2016

oh, thanks for pointing that out.. new one pushed

@martbab
Copy link
Contributor

martbab commented Dec 1, 2016

@tiran I agree, PEP8 errors are often overlooked in the slew of other stuff that gets printed out in the container. I will address this in #293

@MartinBasti MartinBasti self-assigned this Dec 7, 2016
MartinBasti
MartinBasti suggested changes Dec 7, 2016
View reviewed changes
ipaplatform/base/paths.py
@@ -102,6 +102,7 @@ class BasePathNamespace(object):
SMB_CONF = "/etc/samba/smb.conf"
LIMITS_CONF = "/etc/security/limits.conf"
SSH_CONFIG = "/etc/ssh/ssh_config"
SSH_CONFIG_DIR = "/etc/ssh"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please move it one line up, list of paths is sorted alphabetically by path, not by variable name

@MartinBasti
Copy link
Contributor

@tjaalton just one nitpick, otherwise it looks good to me

@MartinBasti MartinBasti added the ack Pull Request approved, can be merged label Jan 5, 2017
@MartinBasti
Copy link
Contributor

Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/0ff12de338a8db32bb10e1b41f32255e7b971b6f

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Jan 5, 2017
@MartinBasti MartinBasti closed this Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartinBasti MartinBasti MartinBasti approved these changes

Assignees

@MartinBasti MartinBasti

Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
Milestone
No milestone
5 participants
@tjaalton @mirielka @tiran @martbab @MartinBasti