Add constants crate #1709
Add constants crate #1709
Conversation
This commit removes the `BaseHttpResponse` struct from the apiserver's mod.rs file Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
|
Forced push includes:
|
I was going to suggest that change. 😎 |
| @@ -87,21 +87,15 @@ use std::fs; | |||
| use std::path::Path; | |||
| use std::process::{self, Command}; | |||
| use std::str::FromStr; | |||
| use constants; | |||
There was a problem hiding this comment.
I would probably import the constant names instead of qualifying them everywhere. (Same in all places...)
| use constants; | |
| use constants::{API_SOCKET, HOST_CTR_BIN, SYSTEMCTL_BIN}; |
There was a problem hiding this comment.
I think this is a case where the opposite is justifiable - qualified constants make it clear that they're not crate-specific, where unqualified ones look like "standard" constants you'd expect in the same file.
There was a problem hiding this comment.
qualified constants make it clear that they're not crate-specific, where unqualified ones look like "standard" constants you'd expect in the same file.
I guess I don't have the expectation that constants are in the same file. To me they're like all other symbols. That is, I have to jump to definition to know where they are. Not a big deal though.
sources/constants/README.md
Outdated
|
|
||
| Current version: 0.1.0 | ||
|
|
||
| This crate contains constants shared accross multiple bottlerocket's crates |
There was a problem hiding this comment.
Across is misspelled :)
I think this should probably read shared across multiple Bottlerocket crates
This commit adds the constants crate, which is shared accross different crates Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
This commit updates the following crates to use the shared constants crate: - apiclient - bootstrap-containers - certdog - corndog - early-boot-config - ecs-settings-applier - host-containers - pluto - schnauzer - service-dog - settings-commiter - static-pods - storewolf - sundog - thar-be-settings - thar-be-updates Signed-off-by: Arnaldo Garcia Rincon <[email protected]>
|
Forced push includes fix in crate's doc string |
| @@ -24,16 +24,11 @@ use std::env; | |||
| use std::ffi::OsStr; | |||
| use std::process::{self, Command}; | |||
| use std::str::FromStr; | |||
| use constants; | |||
There was a problem hiding this comment.
$1.00 if we delete servicedog since we haven't used it for anything in over a year 😀
There was a problem hiding this comment.
You will owe me $2 dollars 🤑
Issue number:
#123
Description of changes:
This change adds the
constantscrate and updates the following crates to use it:As part of this PR, I included a small nit to removed an unused struct imported in
sources/api/apiserver/src/server/mod.rs.Testing done:
aws-k8s-1.21, aws-k8s-1.19, aws-ecs-1:
curl http://localhostfrom within the pod/tasksystemctl statusdidn't show any failuresjournalctl -p noticedidn't show any alarming messageTerms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.