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

Make heapsize and serde support unconditional but without plugins #139

Merged
merged 2 commits into from Jun 10, 2016

Conversation

@nox
Copy link
Member

nox commented Jun 10, 2016

This change is Reviewable

@metajack
Copy link
Contributor

metajack commented Jun 10, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

📌 Commit 1e810da has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

Testing commit 1e810da with merge 38a5481...

bors-servo added a commit that referenced this pull request Jun 10, 2016
Make heapsize and serde support unconditional but without plugins

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/139)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

💔 Test failed - travis

@nox nox force-pushed the nox:stable branch from 1e810da to c255389 Jun 10, 2016
@nox
Copy link
Member Author

nox commented Jun 10, 2016

@bors-servo r=metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

📌 Commit c255389 has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

Testing commit c255389 with merge 43edc5d...

bors-servo added a commit that referenced this pull request Jun 10, 2016
Make heapsize and serde support unconditional but without plugins

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/139)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

💔 Test failed - travis

nox added 2 commits Jun 10, 2016
Support for heapsize and serde are now unconditional.
@nox nox force-pushed the nox:stable branch from c255389 to 85d5f47 Jun 10, 2016
@nox
Copy link
Member Author

nox commented Jun 10, 2016

@bors-servo r=metajack

I can't type.

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

📌 Commit 85d5f47 has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

Testing commit 85d5f47 with merge dd7cbe2...

bors-servo added a commit that referenced this pull request Jun 10, 2016
Make heapsize and serde support unconditional but without plugins

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/139)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 10, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 85d5f47 into servo:master Jun 10, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@nox nox deleted the nox:stable branch Jun 10, 2016
@dbrgn
Copy link

dbrgn commented Jun 22, 2017

Sorry to wake up this thread, but may I ask what the reasoning was for including heapsize as a non-optional feature?

If I understand the note here correctly, it means that that heapsize only works on jemalloc, and with that also euclid and all dependent crates (like lyon)? Or am I misunderstanding something?

@kvark
Copy link
Member

kvark commented Jun 22, 2017

@dbrgn I wasn't a part of the original discussion, but it appears rather straightforward to me to turn them into optional features, even if enabled by default. @nox would you see a problem with that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.