Ultimate-geography: Remove requirements.txt

Created on 10 Apr 2021  路  9Comments  路  Source: anki-geo/ultimate-geography

There's a Dependabot alert caused by an outdated package version in requirements.txt. (It's up to date in Pipenv.lock, though.)

I've asked about this file before: https://github.com/anki-geo/ultimate-geography/pull/364/files#r550776335. Reading a bit more about dependency management in Python, it seems that requirements.txt is on the way out. Python even recommends Pipenv in its own docs

It seems counter-productive to have to keep requirements.txt up to date with Pipenv.lock, especially since GitHub is capable of parsing the latter without problem:

image

Also, I think having the same set-up code running in the CI as the one recommended in CONTRIBUTING.md would make more sense. I don't believe installing Pipenv in the CI would impact the build time, thanks to GitHub Actions' caching system.

chore

Most helpful comment

Looks like UG is still mentioned as a dependent of Brain Brew 馃帀:

image

All 9 comments

Yes I am aware of this silly dependency alert, it is not actually an issue as Brain Brew does not use PyYaml any longer, it is just PyYaml was mistakenly left in the Piplock file during my last update (it was very hard to get rid of it :sob:) I will get rid of it soon, when I update the repo to the latest Brain Brew (note models are in yaml now :wink:)

As for why the requirement file is there in the first place, let me just reiterate: it is not used for dependency installation :sweat_smile: Pipenv is used in Brain Brew, and all repos that currently use it, as the installation guide states.

So why is the requirements.txt file even there then? I was under the impression that this is what Github looks to in order to work out the dependencies of a project, and thus show the pretty dependencies / dependents view, and give us these types of warnings when things need updating. If that is not the case, and it can also look in the Pipfile, then of course we should get rid of it as it is duplication :+1: otherwise I think it serves an important function.

As for why the requirement file is there in the first place, let me just reiterate: it is not used for dependency installation 馃槄 Pipenv is used in Brain Brew, and all repos that currently use it, as the installation guide states.

It's used in the CI:

python -m pip install --upgrade pip
pip install -r requirements.txt

I'll open a PR, then. 馃憤

Yes, it's used in CI. But that is faster, I expect, than installing a Pipenv everytime on the PI :thinking:

If requirements.txt is being removed, of course this should change too. If it is not, is it a big deal? :thinking: No worries either way, from me.

GitHub's CI is fast. I don't think it will make a significant difference. And even if it's 2s slower, I'd much prefer that than to have to remember to keep requirements.txt up to date every time we update Brain Brew in the future. 馃槃

Aha!

image

Okay, it seems you are right and the Pipfile can work for our needs :100: :ok_hand: that is excellent, I now fully support this and will remove the dirty requirements.txt file myself too :laughing:

Sorry, my initial screenshot did not give much context. But you do have access to the dependency graph page of this repo, now, don't you? https://github.com/anki-geo/ultimate-geography/network/dependencies I made you guys owners, so 馃

Looks like UG is still mentioned as a dependent of Brain Brew 馃帀:

image

Sorry, my initial screenshot did not give much context. But you do have access to the dependency graph page of this repo, now, don't you? https://github.com/anki-geo/ultimate-geography/network/dependencies I made you guys owners, so crossed_fingers

Yes, I do have access. But when initially looking at your screenshot I had a brain fart and did not realise the obvious "Dependencies found in Pipfile" text right in front of me :laughing:

Just realised the CI was not running on master. This fixes it: https://github.com/anki-geo/ultimate-geography/commit/b5f4bb39715a18fc01479820cebbace1f1df8c70

Was this page helpful?
0 / 5 - 0 ratings

Related issues

axelboc picture axelboc  路  3Comments

NathanaelRea picture NathanaelRea  路  3Comments

flaheathrenn picture flaheathrenn  路  3Comments

axelboc picture axelboc  路  3Comments

siriak picture siriak  路  3Comments