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

gnrc_tftp: remove module #12906

Merged
merged 2 commits into from
Feb 20, 2020
Merged

gnrc_tftp: remove module #12906

merged 2 commits into from
Feb 20, 2020

Conversation

nmeum
Copy link
Member

@nmeum nmeum commented Dec 9, 2019

Contribution description

Has been deprecated for awhile
(01fc3d8), has known issues and is poorly maintained.

Testing procedure

None.

Issues/PRs references

@miri64
Copy link
Member

miri64 commented Dec 9, 2019

The deprecation message is quite clear when it is to be removed. So this PR has to wait a bit.

@nmeum
Copy link
Member Author

nmeum commented Dec 9, 2019

Sure, just wanted to make sure this isn't forgotten.

@miri64 miri64 added Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Dec 9, 2019
@miri64
Copy link
Member

miri64 commented Dec 9, 2019

I'm not sure if it makes sense to do this already (not likely, but you might to rebase before April so the commit would change), but please remember to also update the LOSTANDFOUND.md

@leandrolanzieri
Copy link
Contributor

@nmeum We can move forward with this now. Please rebase and add this module to LOSTANDFOUND.md.

@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 19, 2020
@nmeum
gnrc_tftp: remove module
0e2a620 
Has been deprecated for awhile (01fc3d8).
@nmeum nmeum force-pushed the pr/remove_gnrc_tftp branch from 5b6d0d6 to 8b1d231 Compare February 20, 2020 11:06
@nmeum
Copy link
Member Author

nmeum commented Feb 20, 2020 •
edited
Loading

@nmeum We can move forward with this now. Please rebase and add this module to LOSTANDFOUND.md.

Done, though I would like to point out that the deprecation note states that this module should be removed (at the latest) after the 2020.04 release. Not sure how the "at the latest" clause is intended to be interpreted though.

miri64
miri64 requested changes Feb 20, 2020
View reviewed changes
@nmeum nmeum force-pushed the pr/remove_gnrc_tftp branch from 8b1d231 to e590bf0 Compare February 20, 2020 13:06
miri64
miri64 approved these changes Feb 20, 2020
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Good from my side. Regarding the deprecation note I technically tend to agree with @nmeum, however this was already deprecated for the 2019.07 release, so I'm not quite sure, why we need to wait for after the 2020.04 release, as it was deprecated now for two releases. @leandrolanzieri you are the release manager for 2020.04, so I leave the final decision to you.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 20, 2020
leandrolanzieri
leandrolanzieri approved these changes Feb 20, 2020
View reviewed changes
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

The module has already been deprecated for 2 releases now, which is the standard time in RIOT. Also the waning seems a bit misleading by stating "..at the latest".

Let's go ahead and remove it for 2020.04. ACK.

@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 20, 2020
@leandrolanzieri leandrolanzieri merged commit ec0e18b into RIOT-OS:master Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

@aabadie aabadie Awaiting requested review from aabadie

@cgundogan cgundogan Awaiting requested review from cgundogan

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

Assignees
No one assigned
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2020.04
Development

Successfully merging this pull request may close these issues.
3 participants
@nmeum @miri64 @leandrolanzieri