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

feat!: Change to native tls #33

Merged
merged 3 commits into from
Aug 2, 2024
Merged

feat!: Change to native tls #33

merged 3 commits into from
Aug 2, 2024

Conversation

emgrav
Copy link
Member

@emgrav emgrav commented Jul 30, 2024

No description provided.

@emgrav emgrav force-pushed the egraven/native-tls branch from 470c74f to 8a2ca84 Compare July 30, 2024 08:39
Copy link

Meow! Coverage

Total: 52.96%

Delta: -0.89%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs145

src/config.rs Outdated Show resolved Hide resolved
Copy link

Meow! Coverage

Total: 52.82%

Delta: -1.03%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs145
src/ldap.rs103-104

src/config.rs Outdated Show resolved Hide resolved
src/ldap.rs Outdated Show resolved Hide resolved
@sirewix
Copy link
Contributor

sirewix commented Jul 30, 2024

To preserve backward-compatability and overall feature richness, I'd suggest to not replace but make 2 features and compile conditionally

@tlater-famedly
Copy link
Contributor

tlater-famedly commented Jul 30, 2024

To preserve backward-compatability and overall feature richness, I'd suggest to not replace but make 2 features and compile conditionally

From the API surface we are maintaining backwards compatibility as-is; nothing changes for a user, except that more certificates now work.

Well, until 174b1e1 - for that commit we probably need to add a config builder or something to maintain backwards compatibility.

@tlater-famedly
Copy link
Contributor

Need to fix up the tests and add one for client certs, but this seems to work in practice for famedly/famedly-sync#6.

sirewix
sirewix previously approved these changes Jul 31, 2024
Copy link

github-actions bot commented Aug 2, 2024

Meow! Coverage

Total: 54.20%

Delta: 0.35%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs162-166, 170

src/config.rs Show resolved Hide resolved
Copy link

github-actions bot commented Aug 2, 2024

Meow! Coverage

Total: 54.20%

Delta: 0.35%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs162-166, 170

src/config.rs Show resolved Hide resolved
Copy link

github-actions bot commented Aug 2, 2024

Meow! Coverage

Total: 54.20%

Delta: 0.35%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs162-166, 170

src/config.rs Show resolved Hide resolved
Copy link

github-actions bot commented Aug 2, 2024

Meow! Coverage

Total: 54.20%

Delta: 0.35%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs162-166, 170

src/config.rs Show resolved Hide resolved
@CLAassistant
Copy link

CLAassistant commented Aug 2, 2024

CLA assistant check
All committers have signed the CLA.

@emgrav emgrav marked this pull request as ready for review August 2, 2024 09:48
@emgrav emgrav requested a review from a team as a code owner August 2, 2024 09:48
Copy link

github-actions bot commented Aug 2, 2024

Meow! Coverage

Total: 54.20%

Delta: 0.35%

🐈‍⬛ Untested Changes 🐈‍⬛
File PathLines
src/config.rs162-166, 170

src/config.rs Show resolved Hide resolved
Copy link
Contributor

@nikzen nikzen left a comment

Choose a reason for hiding this comment

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

LGTM

@tlater-famedly tlater-famedly merged commit 033dd71 into main Aug 2, 2024
3 checks passed
@tlater-famedly tlater-famedly deleted the egraven/native-tls branch August 2, 2024 09:56
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.

5 participants