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

TLSSocket: Remove deprecated connect #12543

Merged
merged 1 commit into from
Mar 4, 2020
Merged

TLSSocket: Remove deprecated connect #12543

merged 1 commit into from
Mar 4, 2020

Conversation

kivaisan
Copy link
Contributor

@kivaisan kivaisan commented Mar 2, 2020

Summary of changes

String based connect has been removed but was still defined in TLSSocket header for offloaded variant.

This is a regression fix for 458957d

Impact of changes

Migration actions required

Documentation


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ARMmbed/mbed-os-wan


@kivaisan kivaisan changed the title Netsocket: Remove deprecated connect TLSSocket: Remove deprecated connect Mar 2, 2020
@ciarmcom ciarmcom requested review from a team March 2, 2020 12:00
@ciarmcom
Copy link
Member

ciarmcom commented Mar 2, 2020

@kivaisan, thank you for your changes.
@ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

kjbracey
kjbracey previously approved these changes Mar 2, 2020
Copy link
Contributor

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

This currently conflicts with #12487, which gives this line an override. That would have caused a compilation error diagnosing the mistake, but I didn't see it because it's inside an #ifdef.

I'll rebase that PR on top of this one.

@mergify mergify bot added needs: CI and removed needs: review labels Mar 2, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Mar 2, 2020

CI started

@mergify mergify bot added needs: work and removed needs: CI labels Mar 2, 2020
@mbed-ci
Copy link

mbed-ci commented Mar 2, 2020

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-pytest

@mergify
Copy link

mergify bot commented Mar 3, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
String based connect has been removed but was still defined in TLSSocket header for offloaded variant.

This is a regression fix for 458957d
@mergify mergify bot dismissed kjbracey’s stale review March 3, 2020 10:09

Pull request has been modified.

@kivaisan
Copy link
Contributor Author

kivaisan commented Mar 3, 2020

Kevin's PR got merged first, so I rebased this with latest master.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 3, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented Mar 3, 2020

Test run: SUCCESS

Summary: 7 of 7 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit 6137c98 into ARMmbed:master Mar 4, 2020
@mergify mergify bot removed the ready for merge label Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants