[services] Tunnelbroker - Add yarn scripts for dev-mode
ClosedPublic
Actions

Authored by • max on May 25 2022, 12:19 PM.

Details

Reviewers

• karol
tomek
ashoat

Commits

rCOMMaea219a0c32c: [services] Tunnelbroker - Add yarn scripts for dev-mode

Summary

To run the tunnelbroker in dev-mode we need to export COMM_SERVICES_DEV_MODE environment variable.
Following the current code approach, add the corresponding commands for the tunnelbroker to package.json.

Linear task: ENG-1101

Test Plan

Run run-tunnelbroker-service-dev-mode and check that the tools::isDevMode() is true.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.May 25 2022, 12:19 PM

• max held this revision as a draft.

Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptMay 25 2022, 12:19 PM

Harbormaster completed remote builds in B9473: Diff 13127.May 25 2022, 12:26 PM

• max published this revision for review.May 25 2022, 1:49 PM

• max edited the summary of this revision. (Show Details)

• max added reviewers: • karol, tomek.

• max added a child revision: D4127: [services] Tunnelbroker - Add config ini file for dev-mode.

• max added a child revision: D4129: [services] Tunnelbroker - Change `ConfigManager.load()` to use config file path as a parameter.May 25 2022, 1:56 PM

• max removed a child revision: D4129: [services] Tunnelbroker - Change `ConfigManager.load()` to use config file path as a parameter.May 25 2022, 1:59 PM

ashoat added inline comments.May 25 2022, 2:47 PM

services/package.json
11 ↗	(On Diff #13127)	Is `export` really necessary here? Can we try this instead?

It looks like this diff is consistent with other places where we use export. We should clarify if we really need them and replace in all the places if possible.

services/package.json
30 ↗	(On Diff #13127)	Deleting the newline wasn't intentional, right?

This revision now requires changes to proceed.May 26 2022, 3:33 AM

In D4126#116852, @palys-swm wrote:

It looks like this diff is consistent with other places where we use export. We should clarify if we really need them and replace in all the places if possible.

I think we can remove all the occurrences in the follow-up for all commands here. I'll create a task for that.

services/package.json
11 ↗	(On Diff #13127)	Is `export` really necessary here? Can we try this instead? The code will work without export here. It was added here to be consistent with the rest of the code in this file. I think we can remove all the occurrences in the follow-up for all commands here. I'll create a task for that.
30 ↗	(On Diff #13127)	Deleting the newline wasn't intentional, right? Seems that we didn't have it before and don't need to add it now, because in JSON format files it's removed by the linter. Also, the new line for the `json` type files is removed by the Prettier.

• karol accepted this revision.May 26 2022, 5:01 AM

• max mentioned this in D4136: [services] Remove `export` from the `package.json` scripts.May 26 2022, 6:05 AM

Removed export usage and stacked on top of the D4136.

• max added a parent revision: D4136: [services] Remove `export` from the `package.json` scripts.May 26 2022, 6:14 AM

In D4126#116852, @palys-swm wrote:

It looks like this diff is consistent with other places where we use export. We should clarify if we really need them and replace in all the places if possible.

Done in D4136. This diff is now stacked on top of it.

services/package.json
11 ↗	(On Diff #13127)	Is `export` really necessary here? Can we try this instead? In a follow-up to this comment, I've created D4136 and stacked this diff on top of it.

Harbormaster completed remote builds in B9492: Diff 13146.May 26 2022, 6:20 AM

Just wanted to say I really appreciate the diff stacking here – this is the perfect way to handle this feedback: unrelated changes in a diff that is stacked before this one

tomek accepted this revision.May 27 2022, 2:20 AM

This revision is now accepted and ready to land.May 27 2022, 2:20 AM

Closed by commit rCOMMaea219a0c32c: [services] Tunnelbroker - Add yarn scripts for dev-mode (authored by • max). · Explain WhyMay 29 2022, 11:26 PM

This revision was automatically updated to reflect the committed changes.

• max mentioned this in rCOMMa09cc35b28fd: [services] Remove `export` from the `package.json` scripts.

• max added a commit: rCOMMaea219a0c32c: [services] Tunnelbroker - Add yarn scripts for dev-mode.