-
Notifications
You must be signed in to change notification settings - Fork 27
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
MNT/STY/TST: Add pre-commit hooks, CI job, standardize style #183
Conversation
74ad6c5
to
ee2644a
Compare
5ad640b
to
548dcea
Compare
if not foundHutch and path.find('xrt')+hostname.find('xrt')>=-1 or path.find('xtod')+hostname.find('xtod')>=-1: | ||
hutch='LFE' #because we have so many names for the same subnet. | ||
foundHutch=True | ||
if (not foundHutch and path.find('xrt')+hostname.find('xrt') >= -1 |
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.
I'm slightly surprised this line passes flake8 with the lack of + spacing
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.
I'm also confused about this...
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.
I liked the specific implementation of the pre-commit a lot
The specific changes all look correct/nondestructive to me
Description
- [ ] bash scripts (hooooo boy)Motivation and Context
The style is whack.
Shellcheck is a much larger effort, and one that we should be careful with. We'll have to go through those more carefully, but for now the CI will only check diff'd files
I spent way too much time trying to grab specific sha's for pre-commit's
--from-ref
and--to-ref
arguments, but the location of that information in thegithub
payload was inconsistent between push and pull-request. At that point I figured I should just get it working.How Has This Been Tested?
Pre-commit vomits all over my terminal repeatedly
Where Has This Been Documented?
This PR