Skip to content

Move requirements file into seperate one#130

Merged
ramceb merged 3 commits intoeclipse-score:mainfrom
qorix-group:MSP_fix_requirements_file
Mar 27, 2026
Merged

Move requirements file into seperate one#130
ramceb merged 3 commits intoeclipse-score:mainfrom
qorix-group:MSP_fix_requirements_file

Conversation

@MaximilianSoerenPollak
Copy link
Contributor

Having all reqs inside python_basics is breaking other modules. These requirements are only needed for testing, and should therefore be moved somewhere more appropriate where they do not break consumers of python_basics.

@MaximilianSoerenPollak
Copy link
Contributor Author

image

Tests are passing

visibility = ["//visibility:public"],
)

compile_pip_requirements(
Copy link
Contributor

Choose a reason for hiding this comment

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

also how does it work when compiled requirements are not parsed by pip and passed as deps to any of the targets in this file?

Looks like adding only compile_pip_requirements with tag=manual has no impact, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does seem that it has 0 impact as it's nowhere used in the dependencies.

And according to that it would be quiet useless.
But that isn't for me to fix, as I don't maintain this module and don't want to delete things that might be required later.
That's why I have only moved it to an appropriate location at the moment.

Having all reqs inside python_basics is breaking other modules.
These requirements are only needed for testing, and should therefore be
moved somewhere more appropriate where they do not break consumers of
python_basics
Copy link
Contributor

@PiotrKorkus PiotrKorkus left a comment

Choose a reason for hiding this comment

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

lgtm, waiting for codeowners to approve

@MaximilianSoerenPollak
Copy link
Contributor Author

@ramceb @castler

Pinging you two as you last worked on this repo and these files.

This patch is needed to fix other repositories that rely on python_basics requirements file.

As Piotr pointed out above, you are actually not using this requirements file in your tests, and purely rely on docs-as-code requirements to get all of the needed things.
I did not fix this, as I wanted to touch as little as is needed for the fix, and to fix this is at your own disgression. You will know best if this is needed or not. I just wanted to make the issue for all consumers of python_basics go away, so those modules work again.

So please have a look, and let me know any questions that may arise.

@ramceb
Copy link
Contributor

ramceb commented Mar 27, 2026

@ramceb @castler

Pinging you two as you last worked on this repo and these files.

This patch is needed to fix other repositories that rely on python_basics requirements file.

As Piotr pointed out above, you are actually not using this requirements file in your tests, and purely rely on docs-as-code requirements to get all of the needed things. I did not fix this, as I wanted to touch as little as is needed for the fix, and to fix this is at your own disgression. You will know best if this is needed or not. I just wanted to make the issue for all consumers of python_basics go away, so those modules work again.

So please have a look, and let me know any questions that may arise.

thanks @MaximilianSoerenPollak I did the cleanup here: #135

Copy link
Contributor

@ramceb ramceb left a comment

Choose a reason for hiding this comment

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

LGTM

@ramceb ramceb merged commit 7c8dd8f into eclipse-score:main Mar 27, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants