Skip to content

Remove deprecated crc32 and std.md5#1800

Merged
andralex merged 1 commit intodlang:masterfrom
Geod24:remove-deprecated-modules
Dec 21, 2013
Merged

Remove deprecated crc32 and std.md5#1800
andralex merged 1 commit intodlang:masterfrom
Geod24:remove-deprecated-modules

Conversation

@Geod24
Copy link
Member

@Geod24 Geod24 commented Dec 21, 2013

This removes two modules that had been deprecated by 6ff6adc (Sep 16, 2012).
I was about to remove only crc32, which should be hard to do, but as md5 was deprecated on the same commit, I included it as well. Any breakage introduced by this would be very easy to fix.

@Geod24
Copy link
Member Author

Geod24 commented Dec 21, 2013

Ping @jmdavis

@andralex
Copy link
Member

What was the deprecation deadline?

@Geod24
Copy link
Member Author

Geod24 commented Dec 21, 2013

It wasn't set.

Looking at both files history, you can see they had been forgotten for the last 15 months, after std.digest was added, except for 2 sed which converted C-Style array to D-Style.
They were also forgotten by recent PR #1766 . I guess they just didn't get grep'd.

So I just removed them directly, as I don't think any code using a recent phobos use them (importing them produces a warning).

@andralex
Copy link
Member

Fine, thanks.

andralex added a commit that referenced this pull request Dec 21, 2013
@andralex andralex merged commit 67342af into dlang:master Dec 21, 2013
@monarchdodra
Copy link
Collaborator

Hum... I was going to suggest we move them in a "deprecated" folder for at least 1 release. There was no actual removal date, and this is an entire module we are talking about, so I suspect a user or two to be surprised by outright removal.

Moving to std.deprecated.crc32 should be more than a fair warning of "this is going to get removed, you need to use something else, NOW".

IMO.

@jmdavis ?

@braddr
Copy link
Member

braddr commented Dec 21, 2013

Tester broke on win32. Why was this merged before the tester could finished chewing on it? As a result, master is now broken too. Please fix.

@Geod24 Geod24 mentioned this pull request Dec 21, 2013
@Geod24
Copy link
Member Author

Geod24 commented Dec 21, 2013

#1802

@jmdavis
Copy link
Member

jmdavis commented Dec 21, 2013

@Geod24 I had forgotten it prior to that pull, because it wasn't in std, but I didn't add it to that pull, because it involved removing files.

@monarchdodra It's been deprecated had a replacement for well over a year now (probably from before deprecated was changed to a warning), and it's a very straightforward code change to fix any code that breaks from it. Also, we don't have a std.deprecated, and I see no point to such a package, because it would still require people to change their code. If they're going to change it, they might as well just change it to use the new stuff. If they really need the old stuff (which I don't think would be the case here), they can just stick with an older release. So, I see no problem with simply removing crc32.d and std.md5. Now, merging it without making sure that the tests pass is another matter, but I see no problem with simply removing them at this point as long as the removal doesn't break the build.

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