Skip to content

Conversation

@tomersa
Copy link
Contributor

@tomersa tomersa commented Oct 4, 2022

No description provided.

@tomersa tomersa added the enhancement New feature or request label Oct 4, 2022
@tomersa tomersa requested a review from d3dave October 4, 2022 09:30
@tomersa tomersa self-assigned this Oct 4, 2022
@tomersa tomersa marked this pull request as ready for review October 4, 2022 09:59
python -m pip install --upgrade pip
pip install -r requirements.txt
pip install -r dev-requirements.txt
pip install --force-reinstall importlib_metadata~=4.8 # Added requirement separately since it stems from an errorneous lint. See the following link for more details: https://github.com/Granulate/granulate-utils/pull/82#discussion_r985745999
Copy link
Contributor

Choose a reason for hiding this comment

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

So why separately and not in dev-requirements? And why --force-reinstall?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the comment wasn't clear, importlib_metadata==5 is installed as it is a requirement of 1 of the packages used by granulate-utils/gLogger. I need importlib_metadata~=4.8 so I must reinstall it in order for it to work.

It is separate from dev-requirements.yml because it should be temporary until they fix the problematic requirements in the link suggested.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it like intel/gprofiler#519?

Copy link
Contributor Author

@tomersa tomersa Oct 6, 2022

Choose a reason for hiding this comment

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

Yes, that's the same issue actually. If you manage to make it work by upgrading flake8 LMK. I'll remove the fix here

Copy link
Contributor

Choose a reason for hiding this comment

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

I managed. Do the same fix here and remove the importlib_metadata thing

@tomersa tomersa requested a review from Jongy October 6, 2022 14:22
@tomersa
Copy link
Contributor Author

tomersa commented Oct 12, 2022

already fixed

@tomersa tomersa closed this Oct 12, 2022
@Jongy
Copy link
Contributor

Jongy commented Oct 12, 2022

already fixed

@tomersa - this is not fixed, you kept the importlib_metadata in requirements.txt instead of upgrading flake8 as we did in gProfiler. Please revert the change done here and instead upgrade flake8

@Jongy Jongy deleted the feature/move_import_to_ci_instead_of_requirements branch October 12, 2022 11:02
@tomersa
Copy link
Contributor Author

tomersa commented Oct 12, 2022

I've checked it using a temp PR and all the CI tests passed:
https://github.com/Granulate/granulate-utils/pull/90/checks

@Jongy Jongy mentioned this pull request Oct 12, 2022
@Jongy
Copy link
Contributor

Jongy commented Oct 12, 2022

Fixed in #92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants