Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hacking fork as dependency and align modules with stackstorm core. #266

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Apr 2, 2024

This set of patches is required as part of the updates needed to support py3.8, py3.9 and py3.10. This PR is a blocking dependency for StackStorm/st2#6157

twine
unittest2
# 202404: Use forked version for flake8 v7.0.0 to align requirements with st2 test-requirements
hacking @ git+https://github.com/nzlosh/hacking@flake8v7
Copy link
Member

@cognifloyd cognifloyd Apr 10, 2024

Choose a reason for hiding this comment

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

What is hacking used for?

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I see it in the old requirements.
Could you submit your branch as an upstream PR?

Copy link
Contributor Author

@nzlosh nzlosh Apr 11, 2024

Choose a reason for hiding this comment

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

I can. In the mid-term I think we should drop hackingcompletely. I'm not sure how likely upstream will be to accept the version bump as OpenStack components rely on hacking and flake8 v7 has breaking changes, of which I have no idea if it'll impact their code base or not. We could move the fork into the StackStorm organisation and pull from there until we have time to address the use of hacking?

Copy link
Member

Choose a reason for hiding this comment

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

What value does hacking provide? I'm inclined to just disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It provides a set of openstack coding style checks. https://docs.openstack.org/hacking/latest/user/hacking.html#styleguide. I assume that these guidelines are used in St2 as a result of its heritage.

Copy link
Member

Choose a reason for hiding this comment

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

hacking is only used for the orquesta repo. Looking through the list of things it enforces, I don't like this one:

  • [H301] Do not import more than one module per line (*)

Copy link
Member

Choose a reason for hiding this comment

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

But whatever. It's not worth the hassle of moving it to another org. Approved.

@guzzijones guzzijones merged commit 197db78 into StackStorm:master Sep 13, 2024
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants