Skip to content
This repository has been archived by the owner on Dec 21, 2020. It is now read-only.

Report errors upstream instead of panicking. #10

Open
wants to merge 105 commits into
base: master
Choose a base branch
from

Conversation

yasushi-saito
Copy link

@yasushi-saito yasushi-saito commented Aug 11, 2017

As a collateral change, remove uses of channels. It doesn't improve performance. Uses of channels make error reporting difficult.

Yaz Saito (MBP) and others added 30 commits August 11, 2017 10:25
…mplicating

the code a lot. In particular, it makes it difficult to report errors.

Removed panic() calls. Instead report errors upstream.
This is a prep to code-share dicom and netdicom better.
Started removing the Parser class. It's empty.
Remove the pipe code. It's not all that useful. The image-dumping code is
subsumed by dicomutil.
Handle undefined-length sequence properly.
…tax.

Defined constants for well-known UID types, such as "Transfer Syntax" and
"SOP Class".
@kofalt
Copy link

kofalt commented Oct 5, 2017

As someone who uses this library, the suggested change sounds great - as you might imagine, I have not reviewed the code yet as it looks like a rather large diff 🙂

Have you considered renaming your fork of the library and keeping it that way? I don't know if @gillesdemey is around anymore, but these changes seem extensive enough to nearly be a new codebase.

@gillesdemey
Copy link
Owner

I'm still around and keeping my eye on this PR! :)

In all honesty, I'm considering deprecating this library since I no longer have time to maintain it and advise users to check out @yasushi-saito 's fork unless some folks are willing to step forward and help out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants