This repository has been archived by the owner on Jul 8, 2021. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Blackify #14
Blackify #14
Changes from 13 commits
1684998
47971c1
026473a
5d3e0ab
f743887
7d9fbb4
36b345f
7a65905
026bf3b
c324549
819f8cd
e3d8194
d2af6b4
d79a304
c1ee611
3be35de
5f154cd
8238eb7
565ebfb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Have not replicated Iris' line removing .gitignore - testing has shown that Black is successfully skipping .gitignore automatically.
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 think Iris also has the line
- export INSTALL_DIR=$(pwd)
within the script section rather than the install section. I don't know if this is relevant or not.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.
It's present here too, but further up - line 10
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.
The current travis run seems to give "No Path provided. Nothing to do 😴" despite passing. I wonder if
- export INSTALL_DIR=$(pwd)
is only applying to the contex of theinstall:
block of code and should be moved down to below line 91 where thescript:
section begins.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.
It seemed safer to declare it a second time. As you've alluded to, it seems that environment variables are independent between the code blocks.
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.
Well, "No Python files are present to be formatted. Nothing to do 😴" seems like an improvement at least.
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.
From local testing, I'm expecting:
All done! ✨ 🍰 ✨
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.
The mystery now is why I get 3 passes locally, but on Travis it doesn't like
__init__.py
...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.
From what I can tell, the only thing that needs changing now is a newline at the end of the
__init__.py
file, and I suppose it's good to know it's picking that up. I'd say this is good to go once that's done.