URL: https://github.com/SSSD/sssd/pull/83 Author: lslebodn Title: #83: TESTS: Check new line at end of file Action: opened
PR body: """ @spbnick Do you have a better idea how to filter out exceptions in `src/tests/whitespace_test` """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/83/head:pr83 git checkout pr83
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
spbnick commented: """ @lslebodn I left one suggestion, if that's not what you needed, could you please specify in which way it should be "better"? """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261919223
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
fidencio commented: """ Also, please, split this patch in two parts: "removing new line at the end of file" and "check new line at the end of file". """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261927606
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ On (21/11/16 04:46), fidencio wrote:
Also, please, split this patch in two parts: "removing new line at the end of file" and "check new line at the end of file".
I prefer to have unit test together with fix.
LS
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/SSSD/sssd/pull/83#issuecomment-261927606
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261929527
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
fidencio commented: """ Okay, then. As long as my preferences are respected during the review my patches we're fine. :-) """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261930622
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
fidencio commented: """ Okay, then. As long as my preferences are respected during the review of my patches we're fine. :-) """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261930622
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ On (21/11/16 04:11), Nikolai Kondrashov wrote:
spbnick commented on this pull request.
@@ -35,3 +35,15 @@ fi
exit found } "
+declare found_file=0 +git ls-files | \
- grep -v "^src/config/testconfigs/noparse.api.conf" | \
- grep -v "^src/tests/cmocka/p11_nssdb/.*db" | \
- while read file; do
test `tail -c 1 $file` && \echo "no newline at eof: $file" && \found_file=1- done
+[ $found_file -eq 1] && exit 1
Another trick is to assign `true` or `false` to `found_file`. Then you can simply write this:
$found_file && exit 1
will do
"--exclude*" options does not work with "--cached" which is a defualt. Unfortuntately, it works only with --others --ignored.
I didn't like that there is "grep -v" twice but single regex would not be very readable and I a not aware of better way how to filter out some lines.
Anyway thank you very much for suggestions.
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261931169
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
spbnick commented: """ Ah, I see. Then you can put your patterns into a variable and check against them in the loop, similarly to the way it's done above in the script. You can use extended globs (with `shopt -s extglob`), or regexes as before.
For globs the test will be if `[[ "$file" != $EXCLUDE_GLOB ]]`, and for regexes it will be `! [[ "$file" ~= $EXCLUDE_REGEX ]]`.
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261934231
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ On (21/11/16 04:59), fidencio wrote:
Okay, then. As long as my preferences are respected during the review of my patches we're fine. :-)
Sure I am not agains separate patches for test and code especialy for bigger features. I just prefer to have it together for small changes. But separate patches would not be a blocker for me even for small changes as in this patch.
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261934892
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
spbnick commented: """ Ah, I see. Then you can put your patterns into a variable and check against them in the loop, similarly to the way it's done above in the script. You can use extended globs (with `shopt -s extglob`), or regexes as before.
For globs the test will be if `[[ "$file" != $EXCLUDE_GLOB ]]`, and for regexes it will be `! [[ "$file" =~ $EXCLUDE_REGEX ]]`.
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261934231
URL: https://github.com/SSSD/sssd/pull/83 Author: lslebodn Title: #83: TESTS: Check new line at end of file Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/83/head:pr83 git checkout pr83
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ I updated patch and also fixed build in parallel directory. """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-262276143
URL: https://github.com/SSSD/sssd/pull/83 Author: lslebodn Title: #83: TESTS: Check new line at end of file Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/83/head:pr83 git checkout pr83
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ Updated version was pushed. """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-262282405
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
spbnick commented: """ Looks good to me! """
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-262285936
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
lslebodn commented: """ On (22/11/16 08:16), Nikolai Kondrashov wrote:
Looks good to me!
* 900778b5afd0143005cfd40cc67ad5086481f7ee
LS
"""
See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-262501226
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/83 Author: lslebodn Title: #83: TESTS: Check new line at end of file Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/83/head:pr83 git checkout pr83
sssd-devel@lists.fedorahosted.org
-
fidencio