Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Example for conditionally loading sources #479

Merged
merged 1 commit into from
Oct 22, 2023
Merged

docs: Example for conditionally loading sources #479

merged 1 commit into from
Oct 22, 2023

Conversation

Elsoberanold
Copy link
Contributor

@Elsoberanold Elsoberanold commented Oct 20, 2023

#475 (comment)


Settings struct has two fields to store the content of files based on content matched from sources. PEM is used as an example of where this could be useful.

Copy link
Collaborator

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing your example! I've revised it a little, if you agree with these changes batch commit them via the Github UI 👍 (or I can do that for you if you like)
 


I'm not sure about placing these in a cert folder? They're just the files for a keypair to support asymmetric cryptography. There is no certificate metadata.

  • I wrote / contributed that kind of PEM content here.
  • You could extract the public key out of those PEM encoded certificates, but I like this example being kept simple as-is, just pointing this out as this example does not cover PEM as it might imply.
  • PEM files can bundle multiple certs, and both public and private key into a single file. They use the header/footer to annotate the base64 encoded content, that is then deserialized IIRC.

files or keys might be more appropriate?


For reference the content of the contributed PEM files can be easily decoded via step crypto key inspect on private.pem and public.pem:

RSA Private Key
RSA Private-Key: (2048 bit)
Modulus:
    ec:ff:b9:3b:0d:d8:7d:02:49:d1:6e:03:87:07:49:45:
    45:a2:d5:c6:0e:7a:7e:ea:43:e5:cf:02:d1:4e:75:b2:
    5c:bc:86:5e:4f:e0:fe:7c:41:0d:0e:99:03:33:e9:4e:
    13:52:ad:a5:e8:56:e0:93:99:d7:77:8f:2c:cd:29:86:
    4b:44:7f:d8:53:d5:a0:74:65:ce:4b:5c:1d:cc:e1:8b:
    96:fa:50:16:89:71:7d:9a:81:d6:c5:ad:83:96:9c:86:
    72:4e:dc:4a:55:55:29:b7:65:8c:ff:40:40:ec:cc:75:
    35:e2:dc:92:54:5a:59:ea:8d:e5:12:20:d9:c1:a8:25:
    b6:d6:73:a1:7e:b5:19:65:ba:81:28:ec:7b:fe:21:20:
    8d:d0:2c:47:b2:b9:58:0c:95:21:78:1b:22:2a:42:02:
    05:96:8c:fd:de:8d:48:eb:97:6d:10:a5:7b:b4:e9:d1:
    90:17:c3:bc:4b:bc:ed:c0:bc:2d:99:d5:09:e5:20:6b:
    86:2b:9b:d4:e7:23:af:e0:d2:52:f2:20:f2:43:b3:52:
    9f:60:42:23:2d:57:c4:46:4e:be:16:d0:17:eb:ef:96:
    70:80:07:c2:a0:33:59:f8:3f:52:fe:b9:59:5f:47:89:
    25:de:6b:32:86:e2:41:11:3c:a8:19:36:a3:15:9c:c7
Public Exponent: 65537 (0x10001)
Private Exponent:
    ad:3f:73:04:bc:66:59:5b:e6:e2:75:ed:96:33:b9:58:
    2e:43:8e:ca:2d:a3:56:4b:a2:3e:c8:49:63:00:5c:01:
    7a:d1:45:d8:83:c4:11:c4:7b:39:34:46:9e:94:c0:24:
    16:f4:05:84:96:87:8b:bb:da:66:2c:3c:39:9f:f0:8e:
    ff:4e:9f:b6:5f:f2:76:4d:20:6e:e1:a0:01:18:d8:77:
    d6:72:3d:41:7c:4f:be:65:c8:2c:5c:6d:2f:18:56:6b:
    e1:fb:cc:05:7a:c3:ba:af:1a:49:2b:fe:a8:77:72:2e:
    ec:a9:5e:5c:89:d5:52:8d:a8:14:a0:5f:eb:4f:29:be:
    9f:fc:d2:84:af:09:e3:42:2b:db:e1:eb:85:14:66:79:
    b6:3d:f8:73:0f:8e:e3:60:4f:ae:40:43:3f:09:e6:f8:
    2d:9f:fa:8d:fd:d2:a9:78:bf:3e:96:6e:fc:87:c3:a9:
    ce:c9:68:19:2e:18:88:78:66:ff:84:42:e4:a1:e8:c6:
    2f:92:49:ff:e7:53:e6:a8:eb:df:28:cf:9b:0a:8f:14:
    ea:c1:b3:bd:09:0c:24:a0:d4:5c:53:df:4d:e2:82:69:
    11:5d:62:06:03:9b:de:02:08:68:1a:32:d3:14:06:df:
    a5:9b:6b:c6:da:d3:6e:28:7d:1b:6b:f2:7e:c7:56:41
Prime #1:
    f7:a5:bb:45:76:4f:45:a0:f1:62:a3:23:a9:db:21:64:
    5b:b8:f2:26:02:9d:f9:5b:23:e2:91:c8:6a:9a:2d:e1:
    a6:73:fa:f1:5c:46:0f:dc:f4:4c:d9:9f:af:94:c0:6a:
    32:90:d7:54:92:45:e2:04:8b:02:4f:3b:0a:b5:ff:9b:
    f4:c6:6a:98:76:1c:96:5f:66:a5:01:0a:b9:c5:40:1f:
    27:33:03:37:64:60:7e:d3:b8:a8:c7:0d:a1:57:87:e3:
    00:0c:c1:77:35:81:d4:47:8e:a5:8d:66:bc:f7:f3:5c:
    b7:67:82:bf:55:63:a6:f8:16:c6:ec:9a:de:8d:e1:3f
Prime #2:
    f4:fe:0c:b7:3f:ff:fc:df:04:40:3d:e2:6e:bf:23:fe:
    fc:8c:bf:fe:4a:c2:cc:ec:b7:e5:3c:1c:8e:ec:93:6b:
    b1:04:e6:ea:6a:35:0e:f9:6e:19:89:84:df:84:cf:f1:
    a4:db:76:fb:79:29:76:fc:82:9b:0a:4b:76:7d:bc:3e:
    85:25:30:74:74:da:d2:a5:35:84:3c:a1:53:4e:9f:d5:
    59:3b:f0:1e:e8:7c:98:5f:eb:d4:ac:a7:80:35:b7:21:
    4d:ea:ee:62:a4:54:73:2b:7e:75:36:de:a6:07:38:32:
    43:9d:2e:96:98:bd:d7:de:d2:49:01:45:c3:aa:5a:79
Exponent #1:
    47:f5:65:44:1a:cb:8f:fc:e3:06:f9:46:6c:9d:9a:c7:
    51:8b:9c:f9:04:7b:a8:b0:1d:ee:40:d4:0e:7d:bc:65:
    3b:fb:a9:68:26:9a:c9:13:37:fd:78:a2:d8:df:0d:46:
    0e:69:5d:d8:5a:24:6a:37:4d:b9:1f:12:95:db:2a:69:
    c3:a7:3f:e4:0b:35:e5:4f:d5:40:8e:db:f1:fc:e9:d3:
    e3:8d:04:1b:3d:54:78:a5:c6:9b:6c:33:7e:b5:33:6b:
    f7:60:bd:7a:89:16:af:7b:17:6c:ed:78:73:e2:4c:59:
    9d:85:3b:4d:a3:5f:30:6e:18:18:37:3a:0c:ff:06:fb
Exponent #2:
    eb:d9:42:7e:8b:53:31:a9:b4:9a:ef:b8:73:6a:f9:09:
    39:31:7a:87:20:8b:a5:e1:e1:2b:02:92:6f:99:1a:56:
    9b:24:9f:f4:5d:68:54:d1:15:07:ea:96:8a:e3:7d:98:
    20:5f:d2:8c:46:d8:ff:1e:19:d1:8d:b8:96:0a:77:55:
    2c:b2:5f:92:4d:08:77:ae:e9:f5:32:b5:0f:d0:ea:17:
    e6:7e:c8:2b:c9:1e:61:46:3e:6f:10:03:74:6e:c1:ac:
    83:29:3e:72:a1:c6:56:d5:31:39:40:28:59:67:2b:d7:
    5f:b6:0a:aa:99:c2:70:f5:a6:34:f7:cf:a4:8c:f3:e9
Coefficient:
    dc:63:c5:75:bf:c8:2d:7c:0f:48:41:5f:1c:4e:51:19:
    66:b6:8e:59:ec:fb:15:6f:41:36:b2:99:1e:99:02:d2:
    cf:bf:64:64:04:ab:f0:1f:0c:bc:99:a0:a6:5a:0c:34:
    f9:4f:df:f2:73:14:0c:f5:58:61:af:f9:4f:29:39:53:
    31:15:86:d6:c0:5c:66:33:c9:8f:8d:93:82:01:15:96:
    7e:f0:ff:79:44:90:4b:f2:3a:9f:5f:1a:f6:66:96:1f:
    f6:61:20:64:8f:2f:f0:fa:f4:d7:74:77:56:a9:29:cb:
    e0:7c:8e:57:e9:bf:11:5d:3b:04:b3:91:8e:36:ba:be
Public Key
RSA Public-Key: (2048 bit)
Modulus:
    ec:ff:b9:3b:0d:d8:7d:02:49:d1:6e:03:87:07:49:45:
    45:a2:d5:c6:0e:7a:7e:ea:43:e5:cf:02:d1:4e:75:b2:
    5c:bc:86:5e:4f:e0:fe:7c:41:0d:0e:99:03:33:e9:4e:
    13:52:ad:a5:e8:56:e0:93:99:d7:77:8f:2c:cd:29:86:
    4b:44:7f:d8:53:d5:a0:74:65:ce:4b:5c:1d:cc:e1:8b:
    96:fa:50:16:89:71:7d:9a:81:d6:c5:ad:83:96:9c:86:
    72:4e:dc:4a:55:55:29:b7:65:8c:ff:40:40:ec:cc:75:
    35:e2:dc:92:54:5a:59:ea:8d:e5:12:20:d9:c1:a8:25:
    b6:d6:73:a1:7e:b5:19:65:ba:81:28:ec:7b:fe:21:20:
    8d:d0:2c:47:b2:b9:58:0c:95:21:78:1b:22:2a:42:02:
    05:96:8c:fd:de:8d:48:eb:97:6d:10:a5:7b:b4:e9:d1:
    90:17:c3:bc:4b:bc:ed:c0:bc:2d:99:d5:09:e5:20:6b:
    86:2b:9b:d4:e7:23:af:e0:d2:52:f2:20:f2:43:b3:52:
    9f:60:42:23:2d:57:c4:46:4e:be:16:d0:17:eb:ef:96:
    70:80:07:c2:a0:33:59:f8:3f:52:fe:b9:59:5f:47:89:
    25:de:6b:32:86:e2:41:11:3c:a8:19:36:a3:15:9c:c7
Exponent: 65537 (0x10001)

examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
Comment on lines 32 to 43

let mut result = Map::new();

let key = match text.contains("PRIVATE") {
true => String::from("private_key"),
false => String::from("public_key")
};

result.insert(
key,
Value::new(uri, ValueKind::String(text.into())),
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Bit more verbosity in the example since documentation of implementing custom formats has been said to be lacking.
  • I've improved your match statement, since the file contents could contain something other than a public key, it's better to match on the type found, otherwise throw an error (also worth documenting an example of).
  • Instead of the String::from(), we can just handle this in the final map insert with .to_owned() which will convert &str to String.
Suggested change
let mut result = Map::new();
let key = match text.contains("PRIVATE") {
true => String::from("private_key"),
false => String::from("public_key")
};
result.insert(
key,
Value::new(uri, ValueKind::String(text.into())),
);
// Store any valid keys into this map, they'll be merged with other sources into the final config map:
let mut result = Map::new();
// Identify the PEM encoded data type by the first occurrence found:
// NOTE: This example is kept simple, multiple or other encoded types are not handled.
let key_type = ["PUBLIC", "PRIVATE"].into_iter().find(|s| text.contains(s));
let key = match key_type {
Some("PRIVATE") => "private_key",
Some("PUBLIC") => "public_key",
// Otherwise fail with an error message (the filename is implicitly appended):
_ => return Err(Box::new(
Error::new(ErrorKind::InvalidData, "PEM file did not contain a Private or Public key")
)),
};
result.insert(
key.to_owned(),
Value::new(uri, ValueKind::String(text.into())),
);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, this code works but it is not optimized for all types of .pem files and can result in unexpected behavior. My intention with this approach was just a simplification to have something quickly working.

Copy link
Contributor Author

@Elsoberanold Elsoberanold Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason this line was give an error:
let key_type = ["PUBLIC", "PRIVATE"].into_iter().find(|s| text.contains(s));

Error:

expected a `Fn<(char,)>` closure, found `str`
the trait `Fn<(char,)>` is not implemented for `str`
the following other types implement trait `Pattern<'a>`:
  &'b str
  &'c &'b str

I had to make this change in order to compile:
let key_type = Vec::<&str>::from(["PUBLIC", "PRIVATE"]).into_iter().find(|s| text.contains(s));

examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
@polarathene polarathene changed the title pem format example docs: Example for conditionally loading sources Oct 21, 2023
@Elsoberanold
Copy link
Contributor Author

Thanks for contributing your example! I've revised it a little, if you agree with these changes batch commit them via the Github UI 👍 (or I can do that for you if you like)  

I'm not sure about placing these in a cert folder? They're just the files for a keypair to support asymmetric cryptography. There is no certificate metadata.

  • I wrote / contributed that kind of PEM content here.
  • You could extract the public key out of those PEM encoded certificates, but I like this example being kept simple as-is, just pointing this out as this example does not cover PEM as it might imply.
  • PEM files can bundle multiple certs, and both public and private key into a single file. They use the header/footer to annotate the base64 encoded content, that is then deserialized IIRC.

files or keys might be more appropriate?

For reference the content of the contributed PEM files can be easily decoded via step crypto key inspect on private.pem and public.pem:

RSA Private Key

RSA Private-Key: (2048 bit)
Modulus:
    ec:ff:b9:3b:0d:d8:7d:02:49:d1:6e:03:87:07:49:45:
    45:a2:d5:c6:0e:7a:7e:ea:43:e5:cf:02:d1:4e:75:b2:
    5c:bc:86:5e:4f:e0:fe:7c:41:0d:0e:99:03:33:e9:4e:
    13:52:ad:a5:e8:56:e0:93:99:d7:77:8f:2c:cd:29:86:
    4b:44:7f:d8:53:d5:a0:74:65:ce:4b:5c:1d:cc:e1:8b:
    96:fa:50:16:89:71:7d:9a:81:d6:c5:ad:83:96:9c:86:
    72:4e:dc:4a:55:55:29:b7:65:8c:ff:40:40:ec:cc:75:
    35:e2:dc:92:54:5a:59:ea:8d:e5:12:20:d9:c1:a8:25:
    b6:d6:73:a1:7e:b5:19:65:ba:81:28:ec:7b:fe:21:20:
    8d:d0:2c:47:b2:b9:58:0c:95:21:78:1b:22:2a:42:02:
    05:96:8c:fd:de:8d:48:eb:97:6d:10:a5:7b:b4:e9:d1:
    90:17:c3:bc:4b:bc:ed:c0:bc:2d:99:d5:09:e5:20:6b:
    86:2b:9b:d4:e7:23:af:e0:d2:52:f2:20:f2:43:b3:52:
    9f:60:42:23:2d:57:c4:46:4e:be:16:d0:17:eb:ef:96:
    70:80:07:c2:a0:33:59:f8:3f:52:fe:b9:59:5f:47:89:
    25:de:6b:32:86:e2:41:11:3c:a8:19:36:a3:15:9c:c7
Public Exponent: 65537 (0x10001)
Private Exponent:
    ad:3f:73:04:bc:66:59:5b:e6:e2:75:ed:96:33:b9:58:
    2e:43:8e:ca:2d:a3:56:4b:a2:3e:c8:49:63:00:5c:01:
    7a:d1:45:d8:83:c4:11:c4:7b:39:34:46:9e:94:c0:24:
    16:f4:05:84:96:87:8b:bb:da:66:2c:3c:39:9f:f0:8e:
    ff:4e:9f:b6:5f:f2:76:4d:20:6e:e1:a0:01:18:d8:77:
    d6:72:3d:41:7c:4f:be:65:c8:2c:5c:6d:2f:18:56:6b:
    e1:fb:cc:05:7a:c3:ba:af:1a:49:2b:fe:a8:77:72:2e:
    ec:a9:5e:5c:89:d5:52:8d:a8:14:a0:5f:eb:4f:29:be:
    9f:fc:d2:84:af:09:e3:42:2b:db:e1:eb:85:14:66:79:
    b6:3d:f8:73:0f:8e:e3:60:4f:ae:40:43:3f:09:e6:f8:
    2d:9f:fa:8d:fd:d2:a9:78:bf:3e:96:6e:fc:87:c3:a9:
    ce:c9:68:19:2e:18:88:78:66:ff:84:42:e4:a1:e8:c6:
    2f:92:49:ff:e7:53:e6:a8:eb:df:28:cf:9b:0a:8f:14:
    ea:c1:b3:bd:09:0c:24:a0:d4:5c:53:df:4d:e2:82:69:
    11:5d:62:06:03:9b:de:02:08:68:1a:32:d3:14:06:df:
    a5:9b:6b:c6:da:d3:6e:28:7d:1b:6b:f2:7e:c7:56:41
Prime #1:
    f7:a5:bb:45:76:4f:45:a0:f1:62:a3:23:a9:db:21:64:
    5b:b8:f2:26:02:9d:f9:5b:23:e2:91:c8:6a:9a:2d:e1:
    a6:73:fa:f1:5c:46:0f:dc:f4:4c:d9:9f:af:94:c0:6a:
    32:90:d7:54:92:45:e2:04:8b:02:4f:3b:0a:b5:ff:9b:
    f4:c6:6a:98:76:1c:96:5f:66:a5:01:0a:b9:c5:40:1f:
    27:33:03:37:64:60:7e:d3:b8:a8:c7:0d:a1:57:87:e3:
    00:0c:c1:77:35:81:d4:47:8e:a5:8d:66:bc:f7:f3:5c:
    b7:67:82:bf:55:63:a6:f8:16:c6:ec:9a:de:8d:e1:3f
Prime #2:
    f4:fe:0c:b7:3f:ff:fc:df:04:40:3d:e2:6e:bf:23:fe:
    fc:8c:bf:fe:4a:c2:cc:ec:b7:e5:3c:1c:8e:ec:93:6b:
    b1:04:e6:ea:6a:35:0e:f9:6e:19:89:84:df:84:cf:f1:
    a4:db:76:fb:79:29:76:fc:82:9b:0a:4b:76:7d:bc:3e:
    85:25:30:74:74:da:d2:a5:35:84:3c:a1:53:4e:9f:d5:
    59:3b:f0:1e:e8:7c:98:5f:eb:d4:ac:a7:80:35:b7:21:
    4d:ea:ee:62:a4:54:73:2b:7e:75:36:de:a6:07:38:32:
    43:9d:2e:96:98:bd:d7:de:d2:49:01:45:c3:aa:5a:79
Exponent #1:
    47:f5:65:44:1a:cb:8f:fc:e3:06:f9:46:6c:9d:9a:c7:
    51:8b:9c:f9:04:7b:a8:b0:1d:ee:40:d4:0e:7d:bc:65:
    3b:fb:a9:68:26:9a:c9:13:37:fd:78:a2:d8:df:0d:46:
    0e:69:5d:d8:5a:24:6a:37:4d:b9:1f:12:95:db:2a:69:
    c3:a7:3f:e4:0b:35:e5:4f:d5:40:8e:db:f1:fc:e9:d3:
    e3:8d:04:1b:3d:54:78:a5:c6:9b:6c:33:7e:b5:33:6b:
    f7:60:bd:7a:89:16:af:7b:17:6c:ed:78:73:e2:4c:59:
    9d:85:3b:4d:a3:5f:30:6e:18:18:37:3a:0c:ff:06:fb
Exponent #2:
    eb:d9:42:7e:8b:53:31:a9:b4:9a:ef:b8:73:6a:f9:09:
    39:31:7a:87:20:8b:a5:e1:e1:2b:02:92:6f:99:1a:56:
    9b:24:9f:f4:5d:68:54:d1:15:07:ea:96:8a:e3:7d:98:
    20:5f:d2:8c:46:d8:ff:1e:19:d1:8d:b8:96:0a:77:55:
    2c:b2:5f:92:4d:08:77:ae:e9:f5:32:b5:0f:d0:ea:17:
    e6:7e:c8:2b:c9:1e:61:46:3e:6f:10:03:74:6e:c1:ac:
    83:29:3e:72:a1:c6:56:d5:31:39:40:28:59:67:2b:d7:
    5f:b6:0a:aa:99:c2:70:f5:a6:34:f7:cf:a4:8c:f3:e9
Coefficient:
    dc:63:c5:75:bf:c8:2d:7c:0f:48:41:5f:1c:4e:51:19:
    66:b6:8e:59:ec:fb:15:6f:41:36:b2:99:1e:99:02:d2:
    cf:bf:64:64:04:ab:f0:1f:0c:bc:99:a0:a6:5a:0c:34:
    f9:4f:df:f2:73:14:0c:f5:58:61:af:f9:4f:29:39:53:
    31:15:86:d6:c0:5c:66:33:c9:8f:8d:93:82:01:15:96:
    7e:f0:ff:79:44:90:4b:f2:3a:9f:5f:1a:f6:66:96:1f:
    f6:61:20:64:8f:2f:f0:fa:f4:d7:74:77:56:a9:29:cb:
    e0:7c:8e:57:e9:bf:11:5d:3b:04:b3:91:8e:36:ba:be

Public Key

RSA Public-Key: (2048 bit)
Modulus:
    ec:ff:b9:3b:0d:d8:7d:02:49:d1:6e:03:87:07:49:45:
    45:a2:d5:c6:0e:7a:7e:ea:43:e5:cf:02:d1:4e:75:b2:
    5c:bc:86:5e:4f:e0:fe:7c:41:0d:0e:99:03:33:e9:4e:
    13:52:ad:a5:e8:56:e0:93:99:d7:77:8f:2c:cd:29:86:
    4b:44:7f:d8:53:d5:a0:74:65:ce:4b:5c:1d:cc:e1:8b:
    96:fa:50:16:89:71:7d:9a:81:d6:c5:ad:83:96:9c:86:
    72:4e:dc:4a:55:55:29:b7:65:8c:ff:40:40:ec:cc:75:
    35:e2:dc:92:54:5a:59:ea:8d:e5:12:20:d9:c1:a8:25:
    b6:d6:73:a1:7e:b5:19:65:ba:81:28:ec:7b:fe:21:20:
    8d:d0:2c:47:b2:b9:58:0c:95:21:78:1b:22:2a:42:02:
    05:96:8c:fd:de:8d:48:eb:97:6d:10:a5:7b:b4:e9:d1:
    90:17:c3:bc:4b:bc:ed:c0:bc:2d:99:d5:09:e5:20:6b:
    86:2b:9b:d4:e7:23:af:e0:d2:52:f2:20:f2:43:b3:52:
    9f:60:42:23:2d:57:c4:46:4e:be:16:d0:17:eb:ef:96:
    70:80:07:c2:a0:33:59:f8:3f:52:fe:b9:59:5f:47:89:
    25:de:6b:32:86:e2:41:11:3c:a8:19:36:a3:15:9c:c7
Exponent: 65537 (0x10001)

I agree with you, perhaps we should rename the example to "custom_file" or "custom_format_from_file". The current "custom_format" example is only applicable to strings, then I think that the main purpose of this should be just an example to demonstrate how to open custom files and use "config::File::new() feature". About folder name, we could rename it to files, it fits better its purpose.

Copy link
Contributor Author

@Elsoberanold Elsoberanold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are just a few lines that still need our attention!

please check my comments

examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks for the feedback and spotting my review mishaps 🙈

I'll apply these fixes, you'll just need to follow-up with a commit afterwards that renames the folders 👍

If you'd like to update the README section on custom format reference, I think yours would be much better to refer to:

See [custom_format](https://github.com/mehcode/config-rs/tree/master/examples/custom_format) example for more information.

You could optionally rename that File::from_str() example folder to custom_str_format / custom_format_from_str or similar.


After that we're good and I can squash merge it all into a single commit.

examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
examples/read_pem_files/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Elsoberanold Elsoberanold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you have suggested, I have changed example name to custom_file_format and cert folder to files.

Copy link
Contributor Author

@Elsoberanold Elsoberanold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is ready to be merged!

Copy link
Collaborator

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the great contribution! ❤️

Copy link
Collaborator

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rustfmt wants changes, I'll handle those 👍

examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
examples/custom_file_format/main.rs Show resolved Hide resolved
examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
examples/custom_file_format/main.rs Outdated Show resolved Hide resolved
@polarathene
Copy link
Collaborator

I'm not familiar with this projects policy on the git check. I could handle it myself with the squash merge commit, but just to be safe I'll ping @matthiasbeyer for his input.


In the meantime, now that everything is passing I can merge this if you satisfy that check.

The easiest way is to squash the commit history yourself down to 1 commit. You then need the end of your commit message to have this:

Signed-off-by: Elsoberanold <[email protected]>

At least I think that will work. It's based off what I did for prior contributions to this repo.

  • First part is your name, but yours is null so it would fallback to the login value (username) I think.
  • Next the Github noreply placeholder email address is the id + login fields of the linked API response AFAIK.

Other than that, the commit message isn't meant to be any longer than 80 chars a line 🙄 (I find this redundant, but these aren't my rules)

@matthiasbeyer
Copy link
Collaborator

Yes, the commit needs signoff. Using the GitHub email really is a bad way of doing this IMO. Your real signoff would be preferred (the -s flag of git commit).
Squash merging is IIRC disabled for this repository, because it destroys valuable history/information.

@polarathene
Copy link
Collaborator

Your real signoff would be preferred (the -s flag of git commit).

My git details are the same as what Github uses, I don't like having a legitimate mail address assigned to commits, I can be reached by other ways, no need for more spam mail 😅

The address I described is one Github assigns to accounts, it's not customized.


Squash merging is IIRC disabled for this repository, because it destroys valuable history/information.

There's nothing valuable from individual commits in this history. It'd be noise to the main history IMO. docker-mailserver has been using squash commit for years now. Much less hassle, context from PRs (which the squash merge commit retains a reference to) is sufficient for us.

I can barely recall needing to dive into individual commits when trawling history, it's usually the PR itself I'm interested in and the discussion that took place behind the feature/decision. Any additional issues referenced, etc.

Like the git commit message line length, they are for concerns that aren't as relevant with modern development 🤔

Either, or. My advice is to squash the history either within the PR or via merge.

@Elsoberanold
Copy link
Contributor Author

Now it is signed! I tryed to squash all commits into a single only but it gives an error

@matthiasbeyer
Copy link
Collaborator

I squashed the changes for you, thanks for contributing! I'll wait until CI is green and merge then!

Co-authored-by: Brennan Kinney <[email protected]>
Signed-off-by: Elsoberanold <[email protected]>
Signed-off-by: Matthias Beyer <[email protected]>
@matthiasbeyer matthiasbeyer merged commit 05f7d1e into mehcode:master Oct 22, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants