Skip to content

Comments

Editorial: Miscellaneous cleanups#834

Merged
anssiko merged 2 commits intowebmachinelearning:mainfrom
inexorabletash:misc-improvements-april2025
Apr 9, 2025
Merged

Editorial: Miscellaneous cleanups#834
anssiko merged 2 commits intowebmachinelearning:mainfrom
inexorabletash:misc-improvements-april2025

Conversation

@inexorabletash
Copy link
Contributor

@inexorabletash inexorabletash commented Apr 3, 2025

  • Don't explicitly "return undefined" from methods; it's implied. The exception would be methods that conditionally return undefined or another value, but WebNN doesn't have any of those. Added lint rule.

  • Avoid "not greater than 0"; just "not 0" (if unsigned), or "less than or equal to 0" (if signed). Added lint rule.

  • "If" must be followed by ", then". Added lint rule.

  • "Otherwise" must be followed by "," or ":". Added lint rule.

  • More consistently refer to a list's items instead of values, etc.

  • Linkfy references to a list's size.

  • Ensure algorithms don't both throw and reject.


Preview | Diff

- Don't explicitly "return undefined" from methods; it's implied. The
  exception would be methods that conditionally return undefined or
  another value, but WebNN doesn't have any of those. Added lint rule.

- Avoid "not greater than 0"; just "not 0" (if unsigned), or "less
  than or equal to 0" (if signed). Added lint rule.

- "If" must be followed by ", then". Added lint rule.

- "Otherwise" must be followed by "," or ":". Added lint rule.

- More consistently refer to a list's items instead of values, etc.

- Linkfy references to a list's size.

- Ensure algorithms don't both throw and reject.
@inexorabletash inexorabletash requested review from fdwr and huningxin April 4, 2025 16:16
@inexorabletash
Copy link
Contributor Author

@huningxin and @fdwr - could you please review?

The "not greater than 0" wording change may be controversial; there's an argument to be made in favor of that phrasing, so I'm happy to change.

Copy link
Collaborator

@fdwr fdwr left a comment

Choose a reason for hiding this comment

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

👍
If author == inexorabletash, then approve. :b

The "not greater than 0" wording change may be controversial
Fewer negations are fine by me - I'd rather see assert(v.size() >= 4) than assert(!(v.size() < 4)).

Copy link
Contributor

@huningxin huningxin left a comment

Choose a reason for hiding this comment

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

LGTM with a nit

@inexorabletash
Copy link
Contributor Author

Thanks, editors! Please hit the "squash and merge" button when you can.

@anssiko anssiko merged commit 2835868 into webmachinelearning:main Apr 9, 2025
2 checks passed
@anssiko
Copy link
Member

anssiko commented Apr 9, 2025

@inexorabletash thanks! Happened to be here, so picked up this one.

github-actions bot added a commit that referenced this pull request Apr 9, 2025
SHA: 2835868
Reason: push, by anssiko

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@inexorabletash inexorabletash deleted the misc-improvements-april2025 branch April 9, 2025 17:57
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