-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Quadlet - fix trailing whitespaces handling #21112
Quadlet - fix trailing whitespaces handling #21112
Conversation
d00660a
to
add2887
Compare
@edsantiago I need your help here. The issue that triggered this PR is a case of a trailing space. As part of the fix, I wrote a test unit file that includes a trailing white space. But, the basic tests are failing because of the trailing space. What should I do? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg, ygalblum The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
add2887
to
cc223b9
Compare
Never mind, I've found the solution |
Remove all trailing white spaces from all lines before the line by line processing Add test Exclude the unit file used for the test from whitespace check Signed-off-by: Ygal Blum <[email protected]>
cc223b9
to
70a53cb
Compare
@vrothberg I change the implementation of the method that clears trims the lines (both because it made the unit test fail and because now it's simpler and cannot return an error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM+
I was working on a different patch to throw an error in this situation. But this looks fine. |
a8c8704
into
containers:main
Belated LGTM2 |
Remove all trailing white spaces from all lines before the line by line processing
Add test
Does this PR introduce a user-facing change?
No
Resolves: #21109