Skip to content

Correct index.lock file deletion and other fixes & formatting#52

Merged
sebdah merged 3 commits intosebdah:masterfrom
JoshLabs:master
Sep 27, 2016
Merged

Correct index.lock file deletion and other fixes & formatting#52
sebdah merged 3 commits intosebdah:masterfrom
JoshLabs:master

Conversation

@sandipagarwal
Copy link
Contributor

@sandipagarwal sandipagarwal commented Sep 23, 2016

The following have been fixed.

  1. index.lock was being deleted and has been resolved.
  2. List (mutable object) was being set as a default of a function argument.
  3. If...then...else reduction.
  4. Code formatting.

@sandipagarwal
Copy link
Contributor Author

@sebdah Can we please get this rolled out. This is important and urgent. I don't know how to mark this a showstopper.

Copy link
Owner

@sebdah sebdah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission @sandipagarwal. I have added a couple of comments on your code.

Sorry for the late response to this, I have been traveling. I'll get this merged and released as soon as the comments are addressed.


import argparse
import os
# import os
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove instead of commenting out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if os.path.exists('.git/index.lock'):
os.remove('.git/index.lock')
# if os.path.exists('.git/index.lock'):
# os.remove('.git/index.lock')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove instead of commenting out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -72,8 +72,8 @@ def main():
sys.exit(0)

# Remove the index.lock when it's exists
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should also be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

try:
if _is_python_file(filename) and \
not _is_ignored(filename, ignored_files):
if _is_python_file(filename) and not _is_ignored(filename, ignored_files):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this done? It seems like this line exceeds 80 chars. Please revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. Editor was configured so.
Done.

@sandipagarwal
Copy link
Contributor Author

Thanks @sebdah. I have fixed all review comments from you.

@sebdah sebdah merged commit b2db938 into sebdah:master Sep 27, 2016
@sebdah
Copy link
Owner

sebdah commented Sep 27, 2016

This has now been released in version 2.2.1.

Thanks again @sandipagarwal for the submission!

@sandipagarwal
Copy link
Contributor Author

Thanks @sebdah. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants