Skip to content

Conversation

albu-diku
Copy link
Contributor

Use local-requirements.txt as the mechanism through which flake8 and pylint and provisioned. Switch over to this and make the lint package installs consistent. Install all deps via pip otherwise changes within code that has external dependencies will fail to lint with missing modules.

@albu-diku albu-diku force-pushed the fix/missing-requirements-for-lint branch from b66eab5 to b4c608a Compare August 1, 2025 10:15
@albu-diku albu-diku changed the title Fix/missing requirements for lint Fix missing requirements for lint Aug 1, 2025
@albu-diku albu-diku marked this pull request as draft August 1, 2025 10:22
@jonasbardino
Copy link
Contributor

jonasbardino commented Aug 5, 2025

Sounds sensible. I think we can go ahead and merge it.

Two slightly related questions:

  1. Do we need to consider the actual order of those three pip install X.txt calls?
  2. Do we really want/need recommended.txt to include a copy of all requirements.txt entries? (not really part of this PR)

As long as we don't have overlapping or conflicting dependencies I suppose order doesn't matter (1), which however leads to (2).
Maybe deciding that we always use pip install requirements.txt optionally followed by pip install recommended.txt and pip install local-requirements.txt would help consistency. What do you think?

@albu-diku
Copy link
Contributor Author

Sounds sensible. I think we can go ahead and merge it.

Two slightly related questions:

1. Do we need to consider the actual order of those three `pip install X.txt` calls?

2. Do we really want/need `recommended.txt` to include a copy of all `requirements.txt` entries? (not really part of this PR)

As long as we don't have overlapping or conflicting dependencies I suppose order doesn't matter (1), which however leads to (2). Maybe deciding that we always use pip install requirements.txt optionally followed by pip install recommended.txt and pip install local-requirements.txt would help consistency. What do you think?

Was about to write directly but... we had the same thought: actually I got bitten by the duplication you mentioned in some of the PRs that need to make sure of flask, where it had to be added across multiple files. Sometihng I found out when I rebased and then had branches explode. I tend to agree that it would simplify matters to be clear which entries should be in which ones (requiring their addition only once) and/or get down to fewer of them if possible.

@jonasbardino
Copy link
Contributor

jonasbardino commented Aug 16, 2025

Sounds sensible. I think we can go ahead and merge it.
Two slightly related questions:

1. Do we need to consider the actual order of those three `pip install X.txt` calls?

2. Do we really want/need `recommended.txt` to include a copy of all `requirements.txt` entries? (not really part of this PR)

As long as we don't have overlapping or conflicting dependencies I suppose order doesn't matter (1), which however leads to (2). Maybe deciding that we always use pip install requirements.txt optionally followed by pip install recommended.txt and pip install local-requirements.txt would help consistency. What do you think?

Was about to write directly but... we had the same thought: actually I got bitten by the duplication you mentioned in some of the PRs that need to make sure of flask, where it had to be added across multiple files. Sometihng I found out when I rebased and then had branches explode. I tend to agree that it would simplify matters to be clear which entries should be in which ones (requiring their addition only once) and/or get down to fewer of them if possible.

Alright, please move the pip install local-requirements.txt bits down as 3rd pip install step then and toggle PR Draft status once ready. Then I'll merge with a follow-up to clean up redundant entries in requirements, recommended and local-requirements.

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.

2 participants