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

Fix bug in legacy depdency check #8418

Closed
wants to merge 2 commits into from
Closed

Conversation

wouterdb
Copy link
Contributor

Description

closes #8405

Self Check:

Strike through any lines that are not applicable (~~line~~) then check the box

  • Attached issue to pull request
  • Changelog entry
  • Type annotations are present
  • Code is clear and sufficiently documented
  • No (preventable) type errors (check using make mypy or make mypy-diff)
  • Sufficient test cases (reproduces the bug/tests the requested feature)
  • Correct, in line with design
  • End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
  • If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see test-fixes for more info)

@wouterdb wouterdb requested a review from sanderr November 27, 2024 10:43
Comment on lines 1302 to 1303
if (not constraint.marker or constraint.marker.evaluate())
and (
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan of the stacked ifs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't even know it existed!

Copy link
Contributor

Choose a reason for hiding this comment

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

Even our DSL has it!

Copy link
Contributor

Choose a reason for hiding this comment

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

(iirc)

@wouterdb wouterdb added the merge-tool-ready This ticket is ready to be merged in label Nov 27, 2024
@inmantaci
Copy link
Contributor

Processing this pull request

inmantaci pushed a commit that referenced this pull request Nov 27, 2024
)

# Description

closes #8405

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [ ] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
- [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)
@inmantaci
Copy link
Contributor

Merged into branches master in 3fe57cb

@inmantaci inmantaci closed this Nov 27, 2024
inmantaci pushed a commit that referenced this pull request Nov 27, 2024
)

# Description

closes #8405

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [ ] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
- [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)
@inmantaci inmantaci deleted the issue/8405-venv_legacy_bug branch November 27, 2024 12:13
@inmantaci
Copy link
Contributor

Processing #8422.

inmantaci pushed a commit that referenced this pull request Nov 27, 2024
)

Pull request opened by the merge tool on behalf of #8418
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-tool-ready This ticket is ready to be merged in
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extras bug in venv check?
3 participants