Page MenuHomePhabricator

[Nix] Package tunnelbroker
AbandonedPublic

Authored by jon on Jul 9 2022, 12:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 30, 9:47 PM
Unknown Object (File)
Sun, Jun 30, 2:49 AM
Unknown Object (File)
Sat, Jun 29, 11:33 PM
Unknown Object (File)
Fri, Jun 28, 1:15 PM
Unknown Object (File)
Fri, Jun 28, 12:04 AM
Unknown Object (File)
Wed, Jun 26, 6:12 AM
Unknown Object (File)
Wed, Jun 26, 6:12 AM
Unknown Object (File)
Wed, Jun 26, 6:12 AM

Details

Reviewers
varun
ashoat
Summary

Package tunnelbroker through nix
Also export a docker image version of the package.

Depends on D4490

Test Plan

nix build .#comm-tunnelbroker

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 9 2022, 12:32 AM
Harbormaster failed remote builds in B10436: Diff 14380!
jon edited the summary of this revision. (Show Details)
nix/tunnelbroker.nix
60 ↗(On Diff #14380)

Line longer than 80 chars. Is there a Nix formatter/style checker/linter we can integrate so I don't have to keep leaving these comments?

jon added inline comments.
nix/tunnelbroker.nix
60 ↗(On Diff #14380)

I can refactor it

varun added inline comments.
nix/tunnelbroker.nix
19 ↗(On Diff #14403)

nit: unnecessary space

65 ↗(On Diff #14403)

nit: "alongside the service"

73 ↗(On Diff #14403)

Doesn't the dockerfile already specify this?

This revision is now accepted and ready to land.Jul 12 2022, 12:20 PM
This revision now requires review to proceed.Jul 12 2022, 12:20 PM
ashoat added inline comments.
nix/tunnelbroker.nix
79 ↗(On Diff #14403)

Extraneous newline

This revision is now accepted and ready to land.Jul 12 2022, 2:38 PM
jon marked an inline comment as done.

Use finalAttrs instead of let block

Add comment about finalAttrs

jon added inline comments.
nix/tunnelbroker.nix
19 ↗(On Diff #14403)

I commonly do this when sections may be not relevant to the package itself, but in addition to it. In this case, docker isn't relevant to building tunnel broker, but it's relevant to exporting a docker image.

If people don't like that white space delineation, then I can remove it.

73 ↗(On Diff #14403)

This isn't using the Dockerfile, it's a docker image with only tunnelbroker and runtime dependencies.

jon marked 2 inline comments as done.

along side -> alongside

Remove use of finalAttrs, wasn't working

With the corrosion addition, this no longer works, going to re-visit at another time

I think you were looking for "Abandon", not "Close"

This revision is now accepted and ready to land.Jul 22 2022, 12:26 PM