Page MenuHomePhabricator

[electron-update-server] Fix minor typo
ClosedPublic

Authored by ashoat on Dec 1 2023, 12:24 PM.
Tags
None
Referenced Files
F3387368: D10143.id34194.diff
Fri, Nov 29, 9:04 AM
Unknown Object (File)
Tue, Nov 26, 3:18 AM
Unknown Object (File)
Mon, Nov 25, 9:43 PM
Unknown Object (File)
Mon, Nov 25, 7:25 PM
Unknown Object (File)
Fri, Nov 8, 10:10 AM
Unknown Object (File)
Wed, Nov 6, 8:41 AM
Unknown Object (File)
Oct 23 2024, 10:58 AM
Unknown Object (File)
Oct 17 2024, 12:08 PM
Subscribers

Details

Summary

Noticed this on the website, figured I'd fix it.

Test Plan

Not sure how to test this, or how to deploy it... @michal, any pointers?

Diff Detail

Repository
rCOMM Comm
Branch
ashoat/e2e
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat requested review of this revision.Dec 1 2023, 1:31 PM

You can just run yarn dev and open the hosted website on http://localhost:80. I checked your typo fix and the website correctly shows our repository and displays the newest desktop version (9).

But (unrelated to this change) by doing that I discovered that actually flow syntax transformation isn't set up for this package and after the flow version update which added types in this file, electron-update-server can't be run (as node doesn't understand flow syntax). Few ways to fix this:

  1. Set up babel flow transform for this package - I don't think it's worth it, this file won't be changing much
  2. Just use // @noflow and remove types
  3. Use flow comments syntax. This for some reason triggers prettier for me, so I had to ignore it on the lines with types.
This revision is now accepted and ready to land.Dec 4 2023, 2:16 AM

Fix the types while we're here

While looking into the issue with Prettier here, I found this comment from somebody on the Flow team:

We will likely officially deprecate it from Flow soon.

I guess we'll have to make some changes when that happens, but this isn't the only place we use Flow's comment syntax (see D10040 and D10004), so we'll have to address that at a later point anyways.

services/electron-update-server/index.js
4 ↗(On Diff #34181)

// prettier-ignore would have been preferable here, but it doesn't seem to work.

This is unfortunate because // eslint-disable only works if Prettier is run from an ESLint context. That means that if some does eg. yarn prettier --write . from the root this file will get changed. But I guess people don't usually do that.

The alternative would be either one of the other options @michal listed, or to disable Prettier for this file entirely via .prettierignore.

16 ↗(On Diff #34181)

Using the "type annotation comments" format seems to avoid the Prettier issue

This revision was automatically updated to reflect the committed changes.