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

Fixed of compilation on latest nightly locally, but still issue on playg… #71

Closed
wants to merge 2 commits into from

Conversation

izderadicka
Copy link
Contributor

@izderadicka izderadicka commented Jun 13, 2018

Fixing #70, works fine with local nightly, but but looks like very latest nightly has changed again.

@izderadicka
Copy link
Contributor Author

Look like it now works with System allocator - I'm really confused by those changes - which allocator is which now and which to use?

@Gankra
Copy link
Contributor

Gankra commented Jun 13, 2018 via email

@RalfJung
Copy link
Member

Seems like #73 did the same thing and hence broke this PR?

@izderadicka
Copy link
Contributor Author

@RalfJung yes I think it's almost the same except two things:

  1. I think there is logical error in Drain as both Iterator and DoubleEndedIterator impl are using next_back from underlying iterator- so both going in same direction - backwards.
  2. I added some tests to vec-final.

Remainig diffs are just formating ( and I used System instead of Global but Global seems to be right name now).
If you're interested in two changes mentioned above let me know I'll pull latest version and leave just these two.

@RalfJung
Copy link
Member

I think there is logical error in Drain as both Iterator and DoubleEndedIterator impl are using next_back from underlying iterator- so both going in same direction - backwards.

That looks like a typo indeed but it was not introduced by #73. I'd prefer if @gankro would take a look.^^

I added some tests to vec-final.

I'm always in favor of tests. :D Do you know if mdbook test will run these tests?

@Gankra
Copy link
Contributor

Gankra commented Jul 17, 2018

CC @steveklabnik on mdbook

@steveklabnik
Copy link
Member

mdbook test runs rustdoc --test on each of your files, so it should.

@RalfJung
Copy link
Member

So @izderadicka would you like to make a new PR with your tests, and with the fix to next_back (that is likely needed to make the tests pass)?

@izderadicka
Copy link
Contributor Author

@RalfJung will look at it next week as I'm off this week. Closing this one.

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.

4 participants