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

new fixes #37

Merged
merged 8 commits into from
Dec 4, 2014
Merged

new fixes #37

merged 8 commits into from
Dec 4, 2014

Conversation

pteek
Copy link
Collaborator

@pteek pteek commented Nov 30, 2014

Thanks @asmeurer for the fixes.

The universal_newlines=True is not needed on windows for now because I had hard coded the proper regexes for it. I will fix this later.

@pteek
Copy link
Collaborator Author

pteek commented Nov 30, 2014

@feross
The version.py file is conflicting. Please merge it manually.

@asmeurer
Copy link

Instead of duplicating the script you can add a symlink. I don't know how Git handles those on windows though.

@pteek
Copy link
Collaborator Author

pteek commented Nov 30, 2014

I don't know how to go about the sym link. How will it work?

@asmeurer
Copy link

Delete the duplicate file and use ln -s spoof-mac scripts/spoof-mac.py (at least if you are Unix. I don't know how to do it on Windows). If you commit this file git should track it as a symlink. I don't know what will happen if you install it. Distutils may install the symlink or it may install a duplicate file. Either one would probably be fine.

@pteek
Copy link
Collaborator Author

pteek commented Dec 1, 2014

I will test on *nix and windows.

On Mon, Dec 1, 2014 at 5:22 AM, Aaron Meurer notifications@github.com
wrote:

Delete the duplicate file and use ln -s spoof-mac scripts/spoof-mac.py
(at least if you are Unix. I don't know how to do it on Windows). If you
commit this file git should track it as a symlink. I don't know what will
happen if you install it. Distutils may install the symlink or it may
install a duplicate file. Either one would probably be fine.


Reply to this email directly or view it on GitHub
#37 (comment).

@feross
Copy link
Owner

feross commented Dec 1, 2014

@pteek While you're at it, try to fix the merge conflict.

Give me a ping when this is ready to be published.

@asmeurer
Copy link

asmeurer commented Dec 2, 2014

I tested randomize in Mac and it works in this branch (and doesn't in master).

@pteek pteek closed this Dec 4, 2014
@pteek pteek reopened this Dec 4, 2014
@pteek pteek closed this Dec 4, 2014
@pteek pteek reopened this Dec 4, 2014
@pteek pteek closed this Dec 4, 2014
@pteek pteek reopened this Dec 4, 2014
pteek added a commit that referenced this pull request Dec 4, 2014
@pteek pteek merged commit 92b2684 into feross:master Dec 4, 2014
@pteek
Copy link
Collaborator Author

pteek commented Dec 4, 2014

@asmeurer

Please clear the branches you are referring to.

@feross
Copy link
Owner

feross commented Dec 5, 2014

@pteek Looks good. But please address my inline comments.

@asmeurer Can you confirm that the current master branch works for you now? Once I hear it does, I'll push a new version to PyPI.

@pteek
Copy link
Collaborator Author

pteek commented Dec 5, 2014

Done.

Next I will do the symlink stuff when I have my Linux machine.

@feross
Copy link
Owner

feross commented Dec 5, 2014

Okay, let's wait for that before publishing a new version.
On Fri, Dec 5, 2014 at 10:41 AM pteek notifications@github.com wrote:

Done.

Next I will do the symlink stuff when I have my Linux machine.


Reply to this email directly or view it on GitHub
#37 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants