Skip to content

Relax version requirements to accept a greater version#130

Merged
d0ugal merged 1 commit into
mkdocs:masterfrom
d0ugal:relax-requirements
Oct 22, 2014
Merged

Relax version requirements to accept a greater version#130
d0ugal merged 1 commit into
mkdocs:masterfrom
d0ugal:relax-requirements

Conversation

@d0ugal

@d0ugal d0ugal commented Aug 22, 2014

Copy link
Copy Markdown
Member

The requirements are currently pinned - this is an issue as it requires very specific versions. This change updates it to be more flexible and allow anything greater than our previous minimum.

Fixes #104

@Keats

Keats commented Sep 5, 2014

Copy link
Copy Markdown

This can make the install fail if one of these modules update and change apis/has bugs (not very likely to happen but you never know (cf things like jashkenas/underscore#1805) and ensure that 2 installs of mkdocs 1.0 (for example) will be identical.

I usually just update all the dependencies once in a while but if other people prefer that way then it's fine

@d0ugal

d0ugal commented Oct 3, 2014

Copy link
Copy Markdown
Member Author

I'm not sure about this - holding off for the moment.

@d0ugal d0ugal closed this Oct 3, 2014
@d0ugal d0ugal reopened this Oct 17, 2014
@d0ugal
d0ugal force-pushed the relax-requirements branch from 004c2ae to 98836e2 Compare October 17, 2014 20:22
@d0ugal d0ugal mentioned this pull request Oct 17, 2014
@d0ugal

d0ugal commented Oct 17, 2014

Copy link
Copy Markdown
Member Author

@tomchristie Could you check this over? I think it is okay, but I'm not 100% sure. Both myself and @sampsyo (see #104) have ran into version issues.

@d0ugal

d0ugal commented Oct 17, 2014

Copy link
Copy Markdown
Member Author

Should this increase the minimum versions? I wasn't sure, so I just opted to go for greater than or equal to what we required before. The specific issue I ran into was something else installed Jinja2 2.7.3 (as a requirement) but MkDocs requires exactly 2.7.1.

@d0ugal
d0ugal force-pushed the relax-requirements branch from 98836e2 to 26e63fb Compare October 17, 2014 20:34
@d0ugal

d0ugal commented Oct 17, 2014

Copy link
Copy Markdown
Member Author

huh, interesting. It looks like the latest python-markdown (2.4.x) has dropped Python 2.6 support (hence the failure on Travis). That might just settle the question of how long we support 2.6.

@d0ugal

d0ugal commented Oct 17, 2014

Copy link
Copy Markdown
Member Author

For anyone else curious, this is when/why they dropped 2.6: Python-Markdown/markdown@44e718e#diff-2eeaed663bd0d25b7e608891384b7298

Comment thread requirements.txt Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps this is supposed to match the constraint in setup.py, below?

Also, it seems to me (could be wrong) that Python 2.6 support was dropped in Markdown 2.5, not 2.4. (In case that's the reason you're requiring <2.4.)

@sampsyo

sampsyo commented Oct 18, 2014

Copy link
Copy Markdown
Contributor

Looks great to me (modulo the above comment)!

@d0ugal
d0ugal force-pushed the relax-requirements branch from 26e63fb to 27b3268 Compare October 18, 2014 07:41
@d0ugal

d0ugal commented Oct 18, 2014

Copy link
Copy Markdown
Member Author

Good spot - after the Python 2.6 error I tried to update it too quickly.

@d0ugal

d0ugal commented Oct 18, 2014

Copy link
Copy Markdown
Member Author

In light of finding this out about python-markdown, I opened #165

@d0ugal d0ugal added this to the 0.10.0 milestone Oct 18, 2014
@dcramer

dcramer commented Oct 21, 2014

Copy link
Copy Markdown

This is how I generally do it, and IMO is a reasonable best practice

@mjtamlyn

Copy link
Copy Markdown

I strongly agree for install_requires - if two projects pin like this a minor point release apart...

requirements.txt I'm guessing is used for tests and I would be fine leaving pinned.

@d0ugal

d0ugal commented Oct 21, 2014

Copy link
Copy Markdown
Member Author

@mjtamlyn I'd be concerned about the unittests being pinned, as I want them to fail if a new version is an issue.

I think I'll go with this for now, it's got to be better than the current situation which breaks things :)

Thanks all!

d0ugal added a commit that referenced this pull request Oct 22, 2014
Relax version requirements to accept a greater version
@d0ugal
d0ugal merged commit e4b9ea0 into mkdocs:master Oct 22, 2014
@d0ugal
d0ugal deleted the relax-requirements branch October 22, 2014 06:31
@sampsyo

sampsyo commented Oct 22, 2014

Copy link
Copy Markdown
Contributor

Awesome! Major props! ✨

@d0ugal
d0ugal restored the relax-requirements branch November 10, 2014 17:29
@d0ugal
d0ugal deleted the relax-requirements branch March 27, 2015 20:56
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.

Update requirements

5 participants