Page MenuHomePhabricator
Differential D8520

Modify NIX and keyserver configuration files to allow node version >= 18
ClosedPublic
Actions

Authored by marcin on Jul 17 2023, 6:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 30, 9:04 PM
Unknown Object (File)
Mon, Mar 24, 6:52 AM
Unknown Object (File)
Mon, Mar 24, 6:52 AM
Unknown Object (File)
Tue, Mar 11, 6:15 PM
Unknown Object (File)
Tue, Mar 11, 4:04 PM
Unknown Object (File)
Mar 6 2025, 1:16 PM
Unknown Object (File)
Mar 5 2025, 5:22 PM
Unknown Object (File)
Mar 4 2025, 10:51 PM
View All Files
Subscribers
tomek

Details

Reviewers
ashoat
• jon
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMM649a14c6bd91: Modify NIX and keyserver configuration files to allow node version >= 18
Summary

This differential applies temporary solution to allow usage of node version >= 18 since it is necessary for current project regarding blob service and big notifications.
The solution is not perfect and the correct one will be implemented in future: https://linear.app/comm/issue/ENG-4373/revert-to-using-nvm-to-lock-into-specific-node-version

Test Plan
  1. run yarn cleaninstall
  2. run nix develop
  3. run node --version and expect the output to match 18.12.1

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin created this revision.Jul 17 2023, 6:28 AM
Owners added a reviewer: Restricted Owners Package.Jul 17 2023, 6:28 AM
Herald added a subscriber: tomek. · View Herald TranscriptJul 17 2023, 6:28 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 17 2023, 6:39 AM
Harbormaster failed remote builds in B20972: Diff 28752!
marcin updated this revision to Diff 28755.Jul 17 2023, 6:40 AM

Remove engines from eslintrc.json in the keyserver

marcin added a child revision: D8521: Enable webcrypto usage in keyserver codebase and unify aes crypto stack across web and the keyserver.Jul 17 2023, 6:45 AM
Harbormaster completed remote builds in B20973: Diff 28755.Jul 17 2023, 6:57 AM
marcin requested review of this revision.Jul 17 2023, 6:57 AM
• jon accepted this revision.Jul 17 2023, 7:23 AM

LGTM if we just want node>=18 (as mentioned in summary)

This revision is now accepted and ready to land.Jul 17 2023, 7:23 AM
ashoat accepted this revision.Jul 17 2023, 7:26 AM
marcin added a child revision: D8650: Remove 3rd node-fetch package and use native API instead.Jul 28 2023, 6:18 AM
marcin removed a child revision: D8521: Enable webcrypto usage in keyserver codebase and unify aes crypto stack across web and the keyserver.
marcin updated this revision to Diff 29810.Aug 10 2023, 7:54 AM

Rebase before landing

Harbormaster completed remote builds in B21688: Diff 29810.Aug 10 2023, 8:13 AM
Closed by commit rCOMM649a14c6bd91: Modify NIX and keyserver configuration files to allow node version >= 18 (authored by marcin). · Explain WhyAug 10 2023, 8:28 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM649a14c6bd91: Modify NIX and keyserver configuration files to allow node version >= 18.

Revision Contents
Changeset List

PathSize
keyserver/
2 lines
2 lines
nix/
4 lines
5 lines

Diff 29810

View Options

keyserver/.nvmrc

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

keyserver/Dockerfile

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

nix/dev-shell.nix

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

nix/overlay.nix

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes