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 silent error on Convert error, fixes #656 #657

Closed
wants to merge 1 commit into from

Conversation

xini
Copy link

@xini xini commented Nov 11, 2024

Description

Manual testing steps

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

@GuySartorelli
Copy link
Member

GuySartorelli commented Nov 11, 2024

There's no description, no testing steps, and no linked issue, and you haven't ticked several of the checkboxes.
You've submitted enough PRs to know how to fill in the template with all the requested information, and to know that if you can't tick the boxes that either means there's something wrong with what you're submitting or you have questions that you need to ask so you can be confident ticking them.

Please fill in the template in full.

@GuySartorelli
Copy link
Member

In any case I think this is not a suitable solution, assuming this is for #656

I'm going to close this for now, as the issue requires more discussion before we can identify if it's even something we want to change.

@xini
Copy link
Author

xini commented Nov 11, 2024

I haven't ticked all the boxes because it is not clear to me whether this needs test coverage and whether it needs to be covered by the docs. I left them open because I feel like that's not my decision.

The title says "fixes #656"

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.

2 participants