Skip to content

Conversation

domodwyer
Copy link
Contributor

Some parts of the code were referencing types behind feature flags, and CI wasn't catching it - this PR corrects both.


  • fix: missing TLS feature gates (1c0eb13)

    Adds missing cfg(tls) annotations to code blocks that require it, allowing
    cargo check --all-targets to pass.
    
  • ci: check all targets with no features (04f6863)

    Adds --all-targets to the "no default features" build, which causes it to
    validate the flags used in #[cfg(test)] gated blocks too.
    

Adds missing cfg(tls) annotations to code blocks that require it,
allowing cargo check --all-targets to pass.
Adds --all-targets to the "no default features" build, which causes it
to validate the flags used in #[cfg(test)] gated blocks too.
@domodwyer domodwyer self-assigned this Mar 29, 2022
@domodwyer domodwyer changed the title build: fix TLS feature flag fix: fix TLS feature flag Mar 29, 2022
@domodwyer domodwyer changed the title fix: fix TLS feature flag fix: TLS feature flag Mar 29, 2022
async fn connect(
&self,
_tls_config: Option<Arc<rustls::ClientConfig>>,
_tls_config: TlsConfig,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was briefly wondering why this code compiled before, but it turns out that this is test code as well and therefore wasn't checked via cargo build --no-default-features. Good catch!

@crepererum crepererum added the automerge Instruct kodiak to merge the PR label Mar 29, 2022
@kodiakhq kodiakhq bot merged commit 89c84f5 into main Mar 29, 2022
@kodiakhq kodiakhq bot deleted the dom/fix-tls-feature-flag branch March 29, 2022 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Instruct kodiak to merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants