-
-
Notifications
You must be signed in to change notification settings - Fork 280
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
Briefed about what are pinned deps. #1341
base: main
Are you sure you want to change the base?
Briefed about what are pinned deps. #1341
Conversation
HI @PrernaSingh587 thanks for the PR, @CJ-Wright I know that you are busy, but would you mind taking a look into this? I want your opinion since #909 was opened by you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general it's really good as an introduction to the subject, it could be interesting to link the issue for time consuming environment builds to (add link to mamba in FAQ)
Explicitly declaring the versions of the dependencies could be advantageous to the quality of the software and to the developers and the open source community that makes up the software ecosystem. | ||
|
||
* Pinning our dependencies may help in avoiding a situation where our software does not builds or runs due to the release of newer versions of the dependencies which are incompatible with our software, consisting some breaking changes. | ||
* Not updating the pinned dependencies each time we upgrade our software and deploy it with newer versions of those dependencies, can result in an older version to hang around longer than it should, which can pose difficulties if these older version have some security issues. These older versions might also be incompatible with some new dependency that is introduced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is kind of confusing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole part, or just the second point? "Not updating the pinned dependencies...."
@viniciusdc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the second one
While its good on many levels to get your versions of dependencies explicit, it might not always be a good choice to pin them in all the cases. | ||
By not pinning the dependencies , especially the non-crucial ones, we provide fewer constraints on the software and make it easier to incorporate into an existing software stack. | ||
Case in point, If ``numpy==1.1.3`` has been specified / pinned for a software and a person with ``numpy 1.1.4`` already there in his system, tries to install this package, they will have to downgrade numpy to meet the package dependencies or create a new Python environment just to use our package. However, if we specify ``numpy>=1.1.3`` ( i.e. not pinning it to a certain version but to a range of versions) , the package will be installed smoothly. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got the point here, but would you mind rephrase this? "already there in his system, tries to install this package, they will have to downgrade" I needed some air here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohkay , I would get back with something better on this part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you check for this one? I have made slight changes to make it look more readable and easily comprehensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a short note: given the strong male bias in tech, I strongly prefer the gender neutral they
over he
. I tried to keep the whole docs gender neutral during the rewrite. If you prefer to use gender specific pronouns, I would mix he
and she
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I would try to keep it gender neutral. I had that in mind before starting to write this but have missed out on it while writing. Thanks for your feedback :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are just some copy edits. You might take a look at https://conda-forge.org/blog/posts/2020-10-02-versions/ for some context on pins.
I am making this comment solely in my personal capacity and am not conveying any rights to any intellectual property of any third parties.
Co-authored-by: Christopher J. 'CJ' Wright <[email protected]>
Co-authored-by: Christopher J. 'CJ' Wright <[email protected]>
@CJ-Wright Yeah, I did some research about this from different sites and wrote that up on the basis of my understanding with help of @viniciusdc . I have read from the link you have provided and made some notes. Can I make some changes in my PR? @viniciusdc ? |
Hi @PrernaSingh587 , yes feel free to make these changes directly to your PR, it's easier to point things out and helps with the discussion. |
I researched and added the following about Pinned Deps and why they can prove to be useful. This is based on my understanding on what pinning a dependency may mean.
Issue Linked : #909
PR Checklist:
src
directory, not indocs