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

- Fixed dependencies ( requirements.txt modified ) #25

Merged
merged 1 commit into from
Jan 12, 2017

Conversation

bougar
Copy link
Contributor

@bougar bougar commented Dec 23, 2016

No description provided.

@ssaavedra ssaavedra changed the title - Fixed dependencies ( requeriments.txt modified ) - Fixed dependencies ( requirements.txt modified ) Dec 27, 2016
@ssaavedra
Copy link
Member

Woops, right, some dependencies are quite outdated (cryptography is even insecure right now)

Apropos, a full list of the dependencies' state can be tracked at: https://requires.io/github/gpul-org/xea-core/requirements/?branch=master

I think it's best if you put a version boundary instead of leaving it unversioned. Mostly because if we want to update a dependency that some of us have already downloaded, if it does not have a minimum version boundary it will not download the latest version. And some issues might arise if some of us have different library versions (and we won't even know that's the root cause of such an issue).

Example:

I have cryptography==1.4.0 downloaded in the virtualenv.
I now execute pip install -r requirements.txt
I still have cryptography==1.4.0 because it fulfills the "cryptography" dependency. However, if the file had "cryptography>=1.7.2, <1.8" as a dependency I would have downloaded the latest cryptography module in that range because of the lower version bound.

I'm happy to merge this as is, I'm just writing to see what you think about it.

@ssaavedra
Copy link
Member

Apropos, a full list of the dependencies' state can be tracked at: https://requires.io/github/gpul-org/xea-core/requirements/?branch=master

In fact, I just submitted a PR to merge a badge about that in the README, so that it's easier to keep track of this whenever we get outdated again. It's #27

@castrinho8
Copy link

I think it's a good idea to put an exact version number ;)

In fact, i think that at this moment could be interesting to update all dependencies (in the frontend we will earn a lot of time if we update react-bootstrap now because we don't have too much components)

@ssaavedra
Copy link
Member

@castrinho8 Either an exact version or at least a boundary (e.g., not fix it to "patch" versions but only "minor"). In pip you can specify that with the = operator. That is, x=1.2.3 is semantically equivalent to x>=1.2.3,<1.3

in the frontend we will earn a lot of time

That's a different issue and a different PR with different package managers. I agree, but that should be discussed elsewhere.

@ssaavedra
Copy link
Member

Related: gpul-org/xea-web#11

@bougar
Copy link
Contributor Author

bougar commented Dec 27, 2016

I am agree with you. The best options is use the exact version, or as you said, at least a boundary.

@ssaavedra
Copy link
Member

Ping?

Please, could you put a version boundary here, so we can get your first commit pushed? ;-)

@bougar
Copy link
Contributor Author

bougar commented Jan 11, 2017

It should be ready now. Could you check it?
I did a rebase on the commit.
I am sorry for the delay.

Copy link
Member

@ssaavedra ssaavedra left a comment

Choose a reason for hiding this comment

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

Nice!

@castrinho8 castrinho8 merged commit 665b68b into gpul-org:master Jan 12, 2017
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