This diff introduces the config for backup URL and refactors the existing identity service config code so that it can be reused between identity and backup.
Details
Details
Added assert_eq!(BACKUP_SOCKET_ADDR, ""); in the test to get the current value of BACKUP_SOCKET_ADDR in the panic message. Then I've run these command and checked the value of BACKUP_SOCKET_ADDR:
- cargo test --release, no file -> backup.commtechnologies.org
- cargo test --release, file -> value in the file
- cargo test, file -> value in the file
- cargo test, no file -> backup.staging.commtechnologies.org
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Also @varun, a question: other facts folders are .gitignored. Do you think it makes sense to do it for this one too?
Comment Actions
yeah we should .gitignore this facts folder too, good call out
| native/native_rust_library/build.rs | ||
|---|---|---|
| 147–150 ↗ | (On Diff #34435) | should we be more specific with these panic messages? |
Comment Actions
Nice! At first glance feels like over-engineering, but it's gonna pay back in the future
| native/native_rust_library/build.rs | ||
|---|---|---|
| 147–150 ↗ | (On Diff #34435) | +1, error message will contain line number but it's better to have it explicitly say Couldn't write backup service config |
Comment Actions
accepting again but please incorporate @bartek 's suggested panic message and .gitignore the facts folder. thanks!
Comment Actions
Worth noting that we don't .gitignore lib/facts. But assuming we're talking about native/facts here, I think it makes sense to .gitignore.
Looks like native/.gitignore has native/facts/alchemy.json covered, but it should probably just list the whole native/facts folder.