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

chore: clean up exception specifiers #13735

Merged
merged 4 commits into from
Mar 12, 2024
Merged

chore: clean up exception specifiers #13735

merged 4 commits into from
Mar 12, 2024

Conversation

arnetheduck
Copy link
Member

Annotating functions explicitly with {.raises: [Exception].} prevents Nim from performing compile-time exception checking and is almost never desired - it does have a tendency to spread through the codebase however, similar to sub-par const correctness in C++.

The reason these annotations might have seemed necessary can be traced to missing exception specifiers in the go imports bumped in this PR.

See
https://status-im.github.io/nim-style-guide/interop.c.html#functions-and-types for background on Nim-go interop and
https://status-im.github.io/nim-style-guide/errors.exceptions.html for more information on how exception tracking can be leveraged to find missing error handling at compile-time.

This change has no runtime effect - it merely makes compile-time error messages more informative or avoids them entirely.

What does the PR do

Affected areas

StatusQ checklist

  • add documentation if necessary (new component, new feature)
  • update sandbox app
    • in case of new component, add new component page
    • in case of new features, add variation to existing component page
    • nice to have: add it to the demo application as well
  • test changes in both light and dark theme?

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it

Cool Spaceship Picture

Annotating functions explicitly with `{.raises: [Exception].}` prevents
Nim from performing compile-time exception checking and is almost never
desired - it does have a tendency to spread through the codebase
however, similar to sub-par const correctness in C++.

The reason these annotations might have seemed necessary can be traced
to missing exception specifiers in the go imports bumped in this PR.

See
https://status-im.github.io/nim-style-guide/interop.c.html#functions-and-types
for background on Nim-go interop and
https://status-im.github.io/nim-style-guide/errors.exceptions.html for
more information on how exception tracking can be leveraged to find
missing error handling at compile-time.

This change has no runtime effect - it merely makes compile-time error
messages more informative or avoids them entirely.
@status-im-auto
Copy link
Member

status-im-auto commented Feb 27, 2024

Jenkins Builds

Click to see older builds (12)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 6603b50 #1 2024-02-27 10:10:56 ~6 min tests/nim 📄log
✔️ 6603b50 #1 2024-02-27 10:11:46 ~7 min macos/aarch64 🍎dmg
✔️ 6603b50 #1 2024-02-27 10:15:21 ~10 min macos/x86_64 🍎dmg
✔️ 6603b50 #1 2024-02-27 10:15:39 ~11 min tests/ui 📄log
✔️ 6603b50 #1 2024-02-27 10:21:31 ~17 min linux/x86_64 📦tgz
✔️ 6603b50 #1 2024-02-27 10:35:51 ~31 min windows/x86_64 💿exe
✔️ 7c61e6e #2 2024-02-27 11:17:29 ~5 min macos/aarch64 🍎dmg
✔️ 7c61e6e #2 2024-02-27 11:17:52 ~5 min tests/nim 📄log
✔️ 7c61e6e #2 2024-02-27 11:21:57 ~10 min tests/ui 📄log
✔️ 7c61e6e #2 2024-02-27 11:24:22 ~12 min macos/x86_64 🍎dmg
✔️ 7c61e6e #2 2024-02-27 11:26:08 ~14 min linux/x86_64 📦tgz
✔️ 7c61e6e #2 2024-02-27 11:48:03 ~36 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
ed58fc7 #3 2024-03-12 11:17:05 ~4 min macos/aarch64 📄log
✔️ ed58fc7 #3 2024-03-12 11:17:52 ~5 min tests/nim 📄log
ed58fc7 #3 2024-03-12 11:18:21 ~6 min macos/x86_64 📄log
ed58fc7 #3 2024-03-12 11:20:30 ~8 min linux/x86_64 📄log
✔️ ed58fc7 #3 2024-03-12 11:23:21 ~11 min tests/ui 📄log
✔️ f927109 #4 2024-03-12 11:35:23 ~6 min tests/nim 📄log
✔️ f927109 #4 2024-03-12 11:35:32 ~6 min macos/x86_64 🍎dmg
✔️ f927109 #4 2024-03-12 11:36:45 ~8 min macos/aarch64 🍎dmg
✔️ f927109 #4 2024-03-12 11:39:03 ~10 min tests/ui 📄log
✔️ f927109 #4 2024-03-12 11:42:21 ~13 min linux/x86_64 📦tgz
✔️ f927109 #4 2024-03-12 11:58:56 ~30 min windows/x86_64 💿exe

oops, forgot the bumps :)
@arnetheduck
Copy link
Member Author

ping @jrainville @endulab - this would benefit from a timely merge giving its conflict-proneness

@arnetheduck arnetheduck merged commit 4272970 into master Mar 12, 2024
8 checks passed
@arnetheduck arnetheduck deleted the eh-specs branch March 12, 2024 15:20
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