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 run export substring check #5575

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

vyasr
Copy link

@vyasr vyasr commented Dec 31, 2024

Description

This PR fixes #5529.

It also adds a constraint to the test requirements file that was previously missing but is required to get a compatible version of py-lief, and it updates the gitignore for vim swap files.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot
Copy link
Contributor

We require contributors to sign our Contributor License Agreement and we don't have one on file for @vyasr.

In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature, merge the PR (conda/infrastructure#1088), and ping the bot to refresh the PR.

@vyasr
Copy link
Author

vyasr commented Dec 31, 2024

I just signed the CLA. Let me know if I need to forward the confirmation email to anyone.

@vyasr
Copy link
Author

vyasr commented Dec 31, 2024

One note that isn't directly related to this PR but did affect me is that the test requirements file is also missing a few additional requirements.

  • pytest: This one should definitely be there to run the tests. I do see it in the requirements-ci.txt file, but I'm not sure what the purpose of requirements.txt is if not to include everything needed to at least run the tests locally as well.
  • pytest-cov: This probably shouldn't be a hard requirement, but it currently is due to the hardcoded addopts in pyproject.toml.
  • binstar_client: I guess this is a legacy pre-anaconda project, so I'm not sure where it should be coming from or if it should still be used here.

@vyasr
Copy link
Author

vyasr commented Dec 31, 2024

I also need a bit of help figuring out the right way to test this. I've verified the result manually, but I couldn't find an easy programmatic way of doing this with conda_build APIs since rendering isn't sufficient so we can't just inspect metadata. The best I could come up with is running the build and then manually extracting and inspecting the metadata of the result. I'm not sure if there is a better way, though.

@@ -12,7 +12,7 @@ menuinst >=2
packaging
pkginfo
psutil
py-lief
py-lief <0.15
Copy link

Choose a reason for hiding this comment

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

#5565 should make this unnecessary

Copy link
Author

Choose a reason for hiding this comment

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

Depending on when PRs get reviewed happy to remove this change or switch up the merge order as is convenient.

Copy link

Choose a reason for hiding this comment

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

Likewise, I could merge or rebase on main and undo this inside #5565 if this gets merged first. Defer to maintainers on what order they'd rather merge in.

@vyasr
Copy link
Author

vyasr commented Jan 2, 2025

@conda-bot check

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 2, 2025
@vyasr vyasr marked this pull request as ready for review January 2, 2025 17:27
@vyasr vyasr requested a review from a team as a code owner January 2, 2025 17:27
Copy link

codspeed-hq bot commented Jan 2, 2025

CodSpeed Performance Report

Merging #5575 will not alter performance

Comparing vyasr:fix/issue_5529 (f7d5093) with main (9c253dc)

Summary

✅ 5 untouched benchmarks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

Ignored run exports are matched by substring and not exactly
3 participants