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

Remove the default allocator function from Document #2242

Closed
wants to merge 1 commit into from

Conversation

tenderlove
Copy link
Member

We expect that XML documents will be of type T_DATA as they are backed
by a C extension. It was possible for people to call allocate on
Document and end up with a T_OBJECT. This commit removes the default
allocator so the only way to get a document object is via the
constructors we've defined.

@codeclimate
Copy link

codeclimate bot commented May 18, 2021

Code Climate has analyzed commit 16ecfc0 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 93.2% (0.0% change).

View more on Code Climate.

@flavorjones
Copy link
Member

Windows tests are failing because of #2241 which I'm working on

@flavorjones
Copy link
Member

Chatted with @tenderlove and it sounds like we want to put this into draft mode until all the C-struct classes either define allocate or undef it.

@flavorjones flavorjones marked this pull request as draft May 18, 2021 21:40
We expect that XML documents will be of type T_DATA as they are backed
by a C extension.  It was possible for people to call `allocate` on
Document and end up with a T_OBJECT.  This commit removes the default
allocator so the only way to get a document object is via the
constructors we've defined.
@flavorjones
Copy link
Member

I've rebased which should address the failing windows tests.

@flavorjones
Copy link
Member

Superseded by #2249

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.

None yet

2 participants