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

resolves icdiff module usage #75

Closed
wants to merge 11 commits into from
Closed

Conversation

vnitinv
Copy link

@vnitinv vnitinv commented May 3, 2016

Now we can do:

from icdiff import diff
diff('/var/tmp/xxx.txt', '/var/tmp/xxx.txt')

@satta satta mentioned this pull request May 3, 2016
@jeffkaufman
Copy link
Owner

The problem with this patch is that installing icdiff is /usr/local/bin/icdiff will stop working.

@vnitinv
Copy link
Author

vnitinv commented Jul 13, 2016

@jeffkaufman I dont see any issue

[root@gls-esx6-vm16 ~]# which icdiff
/usr/bin/which: no icdiff in (/usr/local/rvm/gems/ruby-2.2.2/bin:/usr/local/rvm/gems/ruby-2.2.2@global/bin:/usr/local/rvm/rubies/ruby-2.2.2/bin:/usr/lib64/qt-3.3/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/rvm/bin:/root/bin)

[root@gls-esx6-vm16 ~]# pip install git+https://github.com/vnitinv/icdiff
Collecting git+https://github.com/vnitinv/icdiff
Cloning https://github.com/vnitinv/icdiff to /tmp/pip-H6JjNW-build
Installing collected packages: icdiff
Running setup.py install for icdiff ... done
Successfully installed icdiff-1.8.5
[root@gls-esx6-vm16 ~]# which icdiff
/usr/local/bin/icdiff

image

@jeffkaufman
Copy link
Owner

Right now our primary installation method is people running:

curl -s https://raw.githubusercontent.com/jeffkaufman/icdiff/release-1.8.1/icdiff \
  | sudo tee /usr/local/bin/icdiff > /dev/null \
  && sudo chmod ugo+rx /usr/local/bin/icdiff

It looks to me like this PR will break that?

@peanball
Copy link

This commit breaks recursive diff because the option argument was changed in one place but not all of them.

This is also the version currently available via PyPi and is broken (https://pypi.python.org/pypi/icdiff). I would suggest taking this broken build down from PyPi until Jeff has accepted the pull request. The entry there just says Jeff Kaufman and I guess he didn't publish such a broken version there!

@vnitinv
Copy link
Author

vnitinv commented Jul 18, 2016

@peanball We should rather fix the issue in place of bringing it down from PyPI. The code base in PyPI might have some (or many) bugs, but by putting it in PyPI it made life easier for many.

I have fixed the --recursive issue in this pull request. 77cddd9
Anyways the onus is with Jeff if he want to remove it from PyPI.

@vnitinv
Copy link
Author

vnitinv commented Jun 6, 2017

Will send a fresh pull request.

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

4 participants