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

Include config file in verification buffer if any #1038

Merged
merged 1 commit into from Aug 25, 2016

Conversation

@lunaryorn
Copy link
Contributor

commented Aug 16, 2016

Closes GH-1021 which see.

LGTM, for the record. @cpitclaudel ?

@cpitclaudel

This comment has been minimized.

Copy link
Member

commented Aug 25, 2016

LGTM. I guess (?) you could use a -when-let in there and save the and in path:

,@(-when-let (value (bound-and-true-p config-file-var))

But the behavior would be a bit different (it wouldn't print anything about config files when config-file-var has value nil)

(Unrelated: every time I see an ,@ followed by a a list with a single element inside a when I find myself wanting for a smarter version of ,@ that properly interpolates Maybes)

@lunaryorn

This comment has been minimized.

Copy link
Contributor Author

commented Aug 25, 2016

I'm not sure; config-file-var is a variable poiting to a symbol, not the symbol itself, so I don't think bound-and-true-p would work.

LGTM (because it didn't catch the first one).

@lunaryorn lunaryorn force-pushed the 1021-include-config-files-in-verification branch from c7d7db1 to 5a25cd9 Aug 25, 2016

@lunaryorn lunaryorn closed this Aug 25, 2016

@lunaryorn lunaryorn reopened this Aug 25, 2016

@cpitclaudel

This comment has been minimized.

Copy link
Member

commented Aug 25, 2016

Urgh, forgot that bound-and-true-p was a macro :) LGTM.

@lunaryorn lunaryorn force-pushed the 1021-include-config-files-in-verification branch from 5a25cd9 to bf417e5 Aug 25, 2016

@lunaryorn

This comment has been minimized.

Copy link
Contributor Author

commented Aug 25, 2016

MMh, looks like LGTM.co lost track of this one. I'll use my admin privileges to merge.

@lunaryorn lunaryorn merged commit 8d38a29 into master Aug 25, 2016

4 checks passed

approvals/lgtm this commit looks good
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@lunaryorn lunaryorn deleted the 1021-include-config-files-in-verification branch Aug 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.