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.

Sign up for GitHub

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

Jump to bottom

Linting fixes #1112

Merged
merged 1 commit into from Aug 22, 2022
Merged

Linting fixes #1112

merged 1 commit into from Aug 22, 2022

Conversation

Akasurde
Copy link
Member

@Akasurde Akasurde commented Aug 9, 2022 •
edited

SUMMARY
  • Added whitespace after the keyword
  • Pinned flake8 to 4.0.1 for now
  • Commented ENV PIP_OPTS=--no-build-isolation

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible_runner/main.py
ansible_runner/cleanup.py

@Akasurde Akasurde requested a review from a team as a code owner August 9, 2022 13:15
@github-actions github-actions bot added the needs_triage New item that needs to be triaged label Aug 9, 2022
@Shrews
Copy link
Contributor

Shrews commented Aug 16, 2022

Remove the pypsrp[kerberos,credssp] and pywinrm[kerberos,credssp] entries from tools/requirements.txt to get the integration tests working and we'll merge these two fixes as one change. Those unneeded entries were pulling in pykrb5 which now requires Cython, thus causing the errors.

I'll work on removing other unneeded entries in that and other requirements files for us in a separate PR, if any.

Shrews
Shrews previously approved these changes Aug 17, 2022
View reviewed changes
Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

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

This will probably require a backport. Not sure if that will cause issues.

@Shrews Shrews dismissed their stale review August 17, 2022 16:07

This is likely to cause problems with images depending on the removed libs.

Shrews
Shrews requested changes Aug 17, 2022
View reviewed changes
tools/requirements.txt Outdated Show resolved Hide resolved
@Shrews
Copy link
Contributor

Shrews commented Aug 17, 2022

Have you identified what changed (flake8 update maybe?) to cause these linting issues to appear? It might be worth considering pinning the linting tools to a specific version to prevent this from happening (and then update the version on major releases).

eqrx
eqrx previously requested changes Aug 18, 2022
View reviewed changes
ansible_runner/cleanup.py Show resolved Hide resolved
@eqrx eqrx dismissed their stale review August 18, 2022 19:47

not really about linting

@github-actions github-actions bot added the test Changes to test files label Aug 19, 2022
@Akasurde
Linting fixes
f08a72d 
* Added whitespace after keyword
* Pinned flake8 version to 4.0.1

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@Akasurde Akasurde requested review from Shrews and eqrx August 19, 2022 09:30
Shrews
Shrews approved these changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

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

LGTM. Probably needs backported.

@Akasurde Akasurde merged commit 2d240e0 into ansible:devel Aug 22, 2022
@Akasurde Akasurde deleted the lint branch August 22, 2022 14:33
@sivel sivel removed the needs_triage New item that needs to be triaged label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shrews Shrews Shrews approved these changes

@eqrx eqrx Awaiting requested review from eqrx

Assignees
No one assigned
Labels
test Changes to test files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Akasurde @Shrews @eqrx @sivel