You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason will be displayed to describe this comment to others. Learn more.
@tburrows13 Was wondering why this file was renamed – I noticed while going through the files in the root dir (pattern matching this + Pipfile wouldn't be as easy as it would normally be).
The reason will be displayed to describe this comment to others. Learn more.
I think that I thought at the time that it was more standard to have all lowercase filenames. I have no objections to changing it back.
I don't know much about this topic, but are we even supposed to have a pipfile and a pipfile.lock in our repo? There's no moviepy process for keeping pipfile.lock up-to-date and pipfile doesn't look like it is doing much at all. At one point we were using pipenv for the windows-based testing, but I got rid of that a while ago. Perhaps these files were created for that and we don't need them anymore (#931)?
The reason will be displayed to describe this comment to others. Learn more.
@tburrows13 I only skimmed the linked issue, but yeah, it looks to me like pipenv was meant as pip alternative for those who prefer that. So if we've not kept it updated that's not very helpful, obviously, but I wouldn't remove it just yet. I'll bookmark it as a thing to look into at a later point. Their purpose is to define the packages needed + to pin versions, in any case. I guess they were inspired by package.json and package-lock.json used by npm in JavaScript projects.
But yes, I'd rename the file back. I think I've only ever seen them written Pipfile and Pipfile.lock. Will make a PR, maybe combine it with some minor changes to setup.py I've made on my end. They kind of go together topic-wise, and then it's not just this one tiny change.
The reason will be displayed to describe this comment to others. Learn more.
Actually I just realised the pipenv thing would make for an ideal "good first issue" for someone who uses pipenv more regularly, so will create one for that.
c962808
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.
@tburrows13 Was wondering why this file was renamed – I noticed while going through the files in the root dir (pattern matching this +
Pipfile
wouldn't be as easy as it would normally be).c962808
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 think that I thought at the time that it was more standard to have all lowercase filenames. I have no objections to changing it back.
I don't know much about this topic, but are we even supposed to have a pipfile and a pipfile.lock in our repo? There's no moviepy process for keeping pipfile.lock up-to-date and pipfile doesn't look like it is doing much at all. At one point we were using pipenv for the windows-based testing, but I got rid of that a while ago. Perhaps these files were created for that and we don't need them anymore (#931)?
c962808
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.
@tburrows13 I only skimmed the linked issue, but yeah, it looks to me like
pipenv
was meant aspip
alternative for those who prefer that. So if we've not kept it updated that's not very helpful, obviously, but I wouldn't remove it just yet. I'll bookmark it as a thing to look into at a later point. Their purpose is to define the packages needed + to pin versions, in any case. I guess they were inspired bypackage.json
andpackage-lock.json
used bynpm
in JavaScript projects.But yes, I'd rename the file back. I think I've only ever seen them written
Pipfile
andPipfile.lock
. Will make a PR, maybe combine it with some minor changes tosetup.py
I've made on my end. They kind of go together topic-wise, and then it's not just this one tiny change.c962808
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.
Actually I just realised the pipenv thing would make for an ideal "good first issue" for someone who uses pipenv more regularly, so will create one for that.