ardite / ardite-core

Core library for Ardite services.
MIT License
3 stars 1 forks source link

Hackathon #15

Closed calebmer closed 8 years ago

calebmer commented 8 years ago

Let's code review this and get it into develop.

calebmer commented 8 years ago

CI will need to change to nightly only. Cool with that?

svmnotn commented 8 years ago

due to?

svmnotn commented 8 years ago

I would prefer if it could be built using stable.

calebmer commented 8 years ago

I would too, but using the derive feature of serde requires nightly. My thoughts to justify nightly are as follows:

  1. Writing the deserialization code by hand is way to hard and really should be automated anyway.
  2. There is no impact to the end user if we use nightly. The program still compiles and functions, no users really care.
  3. The code base isn't big enough to justify the problems with the potential instability.
  4. Rust is still very young, some essential features (like compiler plugins, maybe?) have not yet been implemented.
  5. I'm used to using nightly versions in JavaScript 😊
svmnotn commented 8 years ago
  1. Uhmm .. that sounds like the reason as to why graphql-parser isn't finished.
  2. There is an impact. If we use nightly they have to compile with nightly unless they don't use that code. Which I think is not the case.
  3. True, but the end user's might.
  4. Compiler plugins are unstable because rn they are a major hack.
  5. JS != Rust
svmnotn commented 8 years ago

In result, the only reason as to why I want stable is because I want the users to be able to use stable.

calebmer commented 8 years ago

Ok, we should now pass tests on stable. Check out how I did it and tell me if you can think of a better method.

calebmer commented 8 years ago

Implemented all the suggestions. The issue Box<Self> issue still hasn't been resolved though.

calebmer commented 8 years ago

Ok, box is now unboxed and CI is under control.

svmnotn commented 8 years ago

:+1: as long as the dead code is removed.