-
Notifications
You must be signed in to change notification settings - Fork 286
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
Fixes #25332 - Improve readability for content credentials #7790
Conversation
Issues: #25332 |
Don't think that the rubocop failure is related to this change. |
[test katello] |
@sbernhard the change looks good and tested well for me. thanks! One suggestion, can you make the field wrap on newline again? Right now it appears to not honor newlines edit: we can make a new redmine issue for the word wrap as well, up to you on if you want to address it here or not |
Thanks @beav for your comment. I tested this again. For me, it looks like, that the line wrap doesn't happen because it's missing in our test data.: A "enter" (on linux \n) does start a new line. In case there is no \n, it doesn't wrap. I guess, this is the same behavior as before. Do you expect to do a hard line wrap after 80 digits or so? |
@sbernhard can you try with https://getfedora.org/static/429476B4.txt ? That one should have |
I have tested with https://getfedora.org/static/429476B4.txt ?
Tried with Katello von git master and in this version its broken (as you say) but EVEN without this PR. The commit Katello/bastion@8a92521 broke it as it removed:
|
...test fail because something else but pretty sure not because of this PR. |
@sbernhard thanks for the investigation into the newlines. I am looking into the test failure now. |
@beav hrmm, i think i may have introduced that error, however it may not be easily reproducible. I don't mind taking a look |
@jlsherrill I'm re-kicking my dev env anyway, so if you dont mind looking to start, I can jump in and assist or review once I'm back in action |
[test katello] just to see if the test error is transient |
attempting to resolve here: #7796 |
Does someone fix the newline issue? |
@jlsherrill same error, I suppose that is a good thing 👻 @sbernhard I'll create a redmine issue for the newlines. I think this PR is all set after we get jenkins happy |
[test katello] |
issue for newlines: https://projects.theforeman.org/issues/25357 |
@jlsherrill jenkins is still running, but it looks like the error happened again
I'm running the full test suite locally to see if it repros. I was not able to repro when running just the one test. |
[test katello] |
thanks @sbernhard ! |
@sbernhard the issue described should be fixed with this PR: Katello/bastion#238 |
No description provided.