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

use correct exception in parseCmdArg #614

Merged
merged 3 commits into from
Jun 6, 2023
Merged

Conversation

etan-status
Copy link
Contributor

parseCmdArg is expected to raise ValueError but for enr.Record, Node, PrivateKey, and NatConfig, we raise ConfigurationError. Change to ValueError instead.

`parseCmdArg` is expected to raise `ValueError` but for `enr.Record`,
`Node`, `PrivateKey`, and `NatConfig`, we raise `ConfigurationError`.
Change to `ValueError` instead.
@etan-status
Copy link
Contributor Author

Nim devel error was already present before this PR:

/home/runner/work/nim-eth/nim-eth/eth/p2p/discoveryv5/routing_table.nim(488, 14) Error: undeclared identifier: 'shallowCopy'

@etan-status
Copy link
Contributor Author

Nim 1.6 error was already present before this PR:

fatal.nim(54)            sysFatal
asyncfutures2.nim(426)   futureContinue

    Unhandled defect: asyncfutures2.nim(304, 14) `future.cancelcb.isNil` futures returned from `{.async.}` functions must not use `cancelCallback` [AssertionDefect]
  [FAILED] Router should clear all resources when connection future is cancelled

This is due to recent nim-chronos changes that replaced todos with asserts, so the error existed before those changes and is now being highlighted.

@kdeme
Copy link
Contributor

kdeme commented May 31, 2023

This is due to recent nim-chronos changes that replaced todos with asserts, so the error existed before those changes and is now being highlighted.

Right, something I'll have to look into :)

@etan-status etan-status merged commit 99d980c into master Jun 6, 2023
4 of 8 checks passed
@etan-status etan-status deleted the dev/etan/cu-valueerror branch June 6, 2023 20:28
# at your option.
# This file may not be copied, modified, or distributed except according to
# those terms.

Copy link
Member

@arnetheduck arnetheduck Jun 7, 2023

Choose a reason for hiding this comment

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

should probably have slapped push raises: [] on top here

etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Feb 17, 2024
Status Nim style mandates `{.push raises: []}.` at start of modules.
Ensure that's the case so that exceptions are properly tracked.

- https://status-im.github.io/nim-style-guide/errors.exceptions.html
- status-im/nim-eth#614 (comment)
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Feb 17, 2024
Status Nim style mandates `{.push raises: []}.` at start of modules.
Add a CI task to ensure exceptions keep getting properly tracked.

- https://status-im.github.io/nim-style-guide/errors.exceptions.html
- status-im/nim-eth#614 (comment)
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Feb 18, 2024
Status Nim style mandates `{.push raises: []}.` at start of modules.
Ensure that's the case so that exceptions are properly tracked.

- https://status-im.github.io/nim-style-guide/errors.exceptions.html
- status-im/nim-eth#614 (comment)
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Feb 19, 2024
Status Nim style mandates `{.push raises: []}.` at start of modules.
Add a CI task to ensure exceptions keep getting properly tracked.

- https://status-im.github.io/nim-style-guide/errors.exceptions.html
- status-im/nim-eth#614 (comment)
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.

None yet

3 participants