Skip to content

Conversation

@instr3
Copy link
Contributor

@instr3 instr3 commented Sep 30, 2025

Reviewed #339 and resolved the conflicts

The solution of the PR #339 is simple and efficient, and I do not think extra modifications are required once the conflicts are resolved.

Junyan

@craffel
Copy link
Collaborator

craffel commented Oct 5, 2025

This looks good to me, any objections before merging @stefan-baumann @bmcfee ?

@bmcfee
Copy link
Collaborator

bmcfee commented Oct 6, 2025

Seems ok to me. Thanks @instr3 !

@craffel
Copy link
Collaborator

craffel commented Oct 6, 2025

Looks like there's one lint-related error and then a codecov error and possibly an error related to checking valid URLs, with some URLs gone invalid. @instr3 can you fix the lint issue (run black on everything)? @bmcfee any ideas about the codecov thing? I can look into and fix the URLs (probably involves removing them or pointing to archive.org)

@bmcfee
Copy link
Collaborator

bmcfee commented Oct 6, 2025

I think the codecov issue is still out of our control. As I recall, it stems from having migrated from a personal repo to an org, which somehow (?!) broke PR uploads for non-org-members. There were a smattering of auth-related issues with codecov over the past year, and I haven't kept tabs on it lately. Maybe there's a known fix by now?

@craffel
Copy link
Collaborator

craffel commented Oct 6, 2025

Hmm, the problem URLs seem fine for me too so perhaps that's just transient. So let's just fix the lint and we can merge.

@instr3
Copy link
Contributor Author

instr3 commented Oct 6, 2025

Hi @craffel @bmcfee

Thanks for the reminder! I think I have fixed the lint issue. Please tell me if there are more things to fix.

Junyan

@craffel craffel merged commit fa55a92 into mir-evaluation:main Oct 6, 2025
1 of 10 checks passed
@craffel
Copy link
Collaborator

craffel commented Oct 6, 2025

Merged, thans!

@instr3 instr3 deleted the update_key_eval_method_2 branch October 6, 2025 16:04
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.

4 participants