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.

Sign up for GitHub

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

Jump to bottom

civetweb: remove obsolete code #45988

Closed
wants to merge 1 commit into from
Closed

civetweb: remove obsolete code #45988

wants to merge 1 commit into from

Conversation

mbolivar-nordic
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic commented May 25, 2022 •
edited
Loading

OBSOLETE, replaced by: #46746

@mbolivar-nordic
civetweb: remove obsolete code
998444f 
This code has gone unmaintained and bugs continue to be reported
against it. We do not have the resources as a project to maintain this
in "odd fixes" mode, and nobody has stepped up to maintain it [1], so
sadly this must be removed for now.

If anyone would like to see civetweb supported in upstream Zephyr
again, they are welcome to add it back, as long as they promise to
maintain it going forward.

Many thanks to everyone who has contributed to civetweb support in
Zephyr while it was here. So long and thanks for all the fish.

Fixes: zephyrproject-rtos#45807
Fixes: zephyrproject-rtos#43910

[1] https://lists.zephyrproject.org/g/devel/message/8466

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
@mbolivar-nordic mbolivar-nordic added bug The issue is a bug, or the PR is fixing a bug area: civetweb labels May 25, 2022
@zephyrbot
Copy link
Collaborator

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
civetweb zephyrproject-rtos/civetweb@094aeb4 (zephyr) N/A N/A

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@stephanosio stephanosio requested a review from a team May 25, 2022 05:06
@stephanosio
Copy link
Member

stephanosio commented May 25, 2022 •
edited
Loading

FYI @Nukersson @PiotrZierhoffer

@stephanosio stephanosio added the Release Notes To be mentioned in the release notes label May 25, 2022
@KozhinovAlexander
Copy link
Collaborator

KozhinovAlexander commented May 25, 2022 •
edited
Loading

FYI @Nukersson @PiotrZierhoffer

Thank you for pointing on it. I agree. My huge problem with maintenance was no response to pull-requests with civetweb updates.

@KozhinovAlexander
Copy link
Collaborator

KozhinovAlexander commented May 25, 2022 •
edited
Loading

BTW Does somebody working on lean Websocket server solution specific for Zephyr? I am already trying to push it forward within my budget of 1-2 hours a day :)

@gmarull
Copy link
Member

gmarull commented May 25, 2022

In general, I think non-core modules should not even be part of Zephyr west.yaml. They can be maintained outside of Zephyr and applications can pull them as needed. Other examples would be e.g. LVGL.

@KozhinovAlexander
Copy link
Collaborator

In general, I think non-core modules should not even be part of Zephyr west.yaml. They can be maintained outside of Zephyr and applications can pull them as needed. Other examples would be e.g. LVGL.

This approach sounds good to me, cause there is too much ping-pong with integrations steps needed to be done by Zephyr developers. It would be great if newlibc would cover more and more functionality and stay stable, than samples with civetweb etc. could be done. The only question: How could none-core modules be added as none-core samples to Zephyr for sake of showcase etc.?

@stephanosio stephanosio added this to the v3.2.0 milestone May 25, 2022
@mbolivar-nordic mbolivar-nordic added the DNM This PR should not be merged (Do Not Merge) label May 25, 2022
@mbolivar-nordic
Copy link
Contributor Author

Closing this for now given the discussion at the TSC which said we would prefer to deprecate rather than outright remove for v3.1. If nobody steps up to maintain this by the time 3.2 is getting close to release, though, we are going to remove this module.

@mbolivar-nordic mbolivar-nordic mentioned this pull request Jun 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmarull gmarull gmarull approved these changes

@nashif nashif Awaiting requested review from nashif

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@carlescufi carlescufi Awaiting requested review from carlescufi

@dleach02 dleach02 Awaiting requested review from dleach02

Assignees
No one assigned
Labels
area: civetweb area: Documentation area: Modules area: Networking area: Samples Samples bug The issue is a bug, or the PR is fixing a bug DNM This PR should not be merged (Do Not Merge) manifest manifest-civetweb Release Notes To be mentioned in the release notes
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.
5 participants
@mbolivar-nordic @zephyrbot @stephanosio @KozhinovAlexander @gmarull