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

Initial PR for a new structure of exceptions. #9

Closed
wants to merge 16 commits into from

Conversation

KochC
Copy link
Contributor

@KochC KochC commented Mar 24, 2023

Summary

Initial PR for a new structure of exceptions.

Checklist

  • You agree with our CLA
  • Included tests (or is not applicable).
  • Updated documentation (or is not applicable).
  • Used pre-commit hooks to format and lint the code.
  • Upgrade plan implemented / downwards compatibility implemented

@jonas-w
Copy link
Collaborator

jonas-w commented Mar 24, 2023

Extensions? You mean exceptions?

@KochC KochC changed the title Initial PR for a new structure of extensions. Initial PR for a new structure of exceptions. Mar 24, 2023
@KochC
Copy link
Contributor Author

KochC commented Mar 24, 2023

Extensions? You mean exceptions?

yes, sorry for the confusion 😉 Before merging I suggest to discuss if the new structure makes sense, and then continue with some more examples before a final merge.

@jonas-w
Copy link
Collaborator

jonas-w commented Mar 24, 2023

Extensions? You mean exceptions?

yes, sorry for the confusion wink Before merging I suggest to discuss if the new structure makes sense, and then continue with some more examples before a final merge.

No problem ^^. Could you please fix the merge conflict, that the tests can run?

@jonas-w
Copy link
Collaborator

jonas-w commented Mar 24, 2023

LGTM, but ffs should be renamed to something more descriptive.

@nicornk
Copy link
Contributor

nicornk commented Mar 24, 2023

Thanks for the contribution @KochC . I need to review this properly - please keep it open for now.

@jonas-w jonas-w linked an issue Mar 24, 2023 that may be closed by this pull request
4 tasks
@jonas-w
Copy link
Collaborator

jonas-w commented Mar 24, 2023

We will also need deprecation warnings, otherwise it may instantly break code other people wrote.

@nilskch
Copy link
Contributor

nilskch commented Mar 25, 2023

At the end before merging into the main branch, I suggest to squash the commits into one and reference the issue you raised in the commit message by adding "(#5)" at the end of the commit message.

@nicornk nicornk closed this Jan 29, 2024
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.

Export foundry-dev-tool errors such as DatasetHasOpenTransactionError
4 participants