Review of "Nitrokey 3 OTP" version 1

Details Page Preview

Provide a system menu item to retrieve an OTP from a NitroKey. Pick the desired target from the menu and paste from the clipboard.

Extension Homepage
https://gitlab.com/bonzini/gnome-nk3

No comments.

FAQ

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 4 manual_review 1

Shexli found 5 issues that may need reviewer attention.

EGO-X-006 warning

extensions should not use lookupByURL or lookupByUUID for current extension access

Use `this`, `this.getSettings()` or `this.path` instead of `lookupByURL()` or `lookupByUUID()` for the current extension.

`extensionUtils`

  • extension.js:38
    Extension.lookupByURL(import.meta.url)
  • extension.js:174
    Extension.lookupByURL(import.meta.url)

EGO-A-005 manual_review

extensions should not access the clipboard directly

Direct clipboard access via `St.Clipboard.get_default()` requires reviewer scrutiny.

Review Guidelines

  • extension.js:151
    St.Clipboard.get_default()

EGO-A-004 warning

extension files should not contain excessive ungated console logging

File contains 8 ungated console.log/warn/error calls (threshold: 5).

No excessive logging

  • extension.js:48
    console.log(`from nk.py: ${stdout}`)
  • extension.js:61
    console.warn(error)
  • extension.js:116
    console.warn(error)
  • extension.js:124
    console.warn(error)
  • extension.js:157
    console.warn(error)
  • extension.js:186
    console.warn(`gudev not available: ${error}`)
  • extension.js:216
    console.warn(result['message'])
  • extension.js:244
    console.warn(error)

EGO-L-005 warning

owned object references should be released in disable()

Owned references that are cleaned up in `disable()` should also be released with `null` or `undefined`.

Destroy all objects

  • extension.js:162
    this.menuItem = new PopupMenu.PopupMenuItem(this._label, false)

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • extension.js:163
    this.menuItem.connect('activate', this._clicked.bind(this))
  • extension.js:184
    this._client.connect('uevent', this._onUEvent.bind(this))

All Versions

Version Status
4 Active
3 Rejected
2 Rejected
1 Rejected

Previous Reviews on this Version

SriramRamkrishna rejected
You'll need to do the proper clean up when the extension gets disabled. Can you look at the output of shexli and clean them up and then resubmit?