-
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
ledgerblue installed after running make #4
base: master
Are you sure you want to change the base?
Conversation
@@ -117,6 +117,7 @@ clean: | |||
|
|||
prepare: | |||
@mkdir -p bin obj debug dep | |||
pip install -r requirements.pip |
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.
This is the only actual change in this file ;)
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.
then why don't you remove the other changes in the commit? they are just noise
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.
@knocte The other changes are removing the "noise" left here through the time. Unfortunately atm I don't have any editor set up to leave that "noise" in files I edit. If You can't use this PR because of that feel free to reuse the change We discuss here in Your own, new PR.
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 not the maintainer of this project, I just commented to help you. Maybe your PR didn't get merged because of this? You don't make style changes in the same commit as where you're fixing a problem, if you want your PR merged. Read this http://tirania.org/blog/archive/2010/Dec-31.html
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.
Anyway there are now conflicts, so you would need to rebase.
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.
RE @knocte -- #4 (comment) :
Ty, for advise and link. I'll definitely read it, it looks interesting.
Checking the conflict probably tonight
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.
Actually the reason for the need of a rebase is because of including style changes ;) if they hadn't been there, then this PR wouldn't have had conflicts ;)
This will make it easier for newcomers to play