Skip to content

Require requests >= 2.4#147

Merged
anarcat merged 3 commits intolinkchecker:masterfrom
fd0:fix-requirements
Apr 15, 2018
Merged

Require requests >= 2.4#147
anarcat merged 3 commits intolinkchecker:masterfrom
fd0:fix-requirements

Conversation

@fd0
Copy link
Copy Markdown
Contributor

@fd0 fd0 commented Apr 13, 2018

This is already in the code in linkcheck/__init__.py, update the requirements and setup file so it can be installed via pip.

This is already in the code in `linkcheck/__init__.py`, update the
requirements and setup file so it can be installed via `pip`.
Copy link
Copy Markdown
Contributor

@anarcat anarcat left a comment

Choose a reason for hiding this comment

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

i may have lost track but in #66 we deliberately limited the dependency to <2.15 because of a compatibility problem described in #76. but considering tests pass here I'm not sure what's going on here...

shouldn't we keep the <2.15 dependency? what actually happens when you try to install through pip now?

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 13, 2018

and yes, this is a mess, sorry!

@fd0
Copy link
Copy Markdown
Contributor Author

fd0 commented Apr 13, 2018

It fails when installed via pip on Ubuntu Trusty (that's what Travis uses):

I have no idea if it'd work when keeping other constraint, sorry. I'm just copy&pasting stuff to get it working for the restic website ;)

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 13, 2018

right okay. i took the liberty of editing your branch to restore the other limit, could you test the result and see if it works for you?

@fd0
Copy link
Copy Markdown
Contributor Author

fd0 commented Apr 13, 2018

Sure

@fd0
Copy link
Copy Markdown
Contributor Author

fd0 commented Apr 13, 2018

Seems to work, pip installs a newer version of requests :)

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 13, 2018

awesome, LGTM then.. :) glad to see you use linkchecker and thanks for the contribution!

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 13, 2018

( @mgedmin it would be nice to cleanup those requirements and specify them just in one place... do you have a quick trick up your sleeves on how to do that? i always wonder why we need the .txt file in the first place...)

@fd0
Copy link
Copy Markdown
Contributor Author

fd0 commented Apr 13, 2018

Cool, I like linkchecker, it does what it should. But I spent two hours fighting the old Python 2.7 (without SNI support 😞) on Travis on Ubuntu Trusty. That was not fun at all ☹️ (not your fault though, thanks for your work!)

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 13, 2018

yeah, this thing is really showing its age. this is only what you see as a user, as a developer there are much worse horrors lurking below, e.g. #119

@fd0
Copy link
Copy Markdown
Contributor Author

fd0 commented Apr 14, 2018

WTF? There's an HTML parser in this project? Written in C? Oh well. That was... unexpected...

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 14, 2018 via email

Copy link
Copy Markdown
Contributor

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

LGTM

@mgedmin
Copy link
Copy Markdown
Contributor

mgedmin commented Apr 15, 2018

TBH I think requirements.txt is not necessary for us and can be safely deleted.

It is currently used in tox.ini (redundant, except for the 'argcomplete' dependency that tox wouldn't otherwise pick up as it's missing from install_requires), and mentioned in a couple of files in doc/.

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 15, 2018

@mgedmin let's remove it then, do you mind filing a pr?

@anarcat anarcat merged commit 268bf16 into linkchecker:master Apr 15, 2018
@anarcat anarcat mentioned this pull request Apr 15, 2018
@mgedmin
Copy link
Copy Markdown
Contributor

mgedmin commented Apr 15, 2018

Then I would have to come up with documentation updates... I don't have the energy for that at the moment. :(

@anarcat
Copy link
Copy Markdown
Contributor

anarcat commented Apr 16, 2018

i understand, no problem.

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.

3 participants