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

Transpile regression #2714

Closed
wants to merge 2 commits into from
Closed

Transpile regression #2714

wants to merge 2 commits into from

Conversation

Kukovec
Copy link
Collaborator

@Kukovec Kukovec commented Aug 28, 2023

  • Tests added for any new code
  • Ran make fmt-fix (or had formatting run automatically on all files edited)
  • Documentation added for any new functionality
  • Entries added to ./unreleased/ for any new functionality

Regression test for #2713

@Kukovec Kukovec requested a review from shonfeder as a code owner August 28, 2023 15:23
@codecov-commenter
Copy link

Codecov Report

Merging #2714 (557c4b3) into main (6bb36ac) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2714      +/-   ##
==========================================
- Coverage   78.52%   78.50%   -0.02%     
==========================================
  Files         461      461              
  Lines       15885    15885              
  Branches     2562     2562              
==========================================
- Hits        12473    12470       -3     
- Misses       3412     3415       +3     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@thpani thpani left a comment

Choose a reason for hiding this comment

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

Why are we adding a passing regression test when the issue (#2713) is still open?

Would be useful to record this context in the PR description.

@Kukovec
Copy link
Collaborator Author

Kukovec commented Aug 29, 2023

Why are we adding a passing regression test when the issue (#2713) is still open?

Would be useful to record this context in the PR description.

For context: Shon and I were discussing this on slack, since he couldn't reproduce it on his device. We made a PR specifically so that CI would run it to see if it was something wrong with my environment, or whether there was a code issue. Given that it's passing, it must be the former.

@shonfeder
Copy link
Contributor

Thanks for providing the context, Jure!

I agree with Thomas that it would help us all collaborate and save time to ensure every or had at least a sentence or two (but as much as needed) noting the context.

@shonfeder shonfeder closed this Aug 29, 2023
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