search

ardite / ardite-core

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

Feature: validate query method #9

Closed calebmer closed 8 years ago

calebmer commented 8 years ago

I'm doing some work on ardite-rest, as I find useful methods I'll implement them. validate_value is going to suck to implement…

calebmer commented 8 years ago

This is not ready to be merged btw

svmnotn commented 8 years ago

can we have an internal rule where if the module is, lets say over 200 lines, to represent too long (tests not included unless they are over ... idk what seems too long for tests? 300?) we split it into its own module?

Since that does seem to be what happened to schema.

calebmer commented 8 years ago

I don't think so. For javascript I'd split stuff up into lots of small files, but for all the codes I've seen, large files seem to be preferred in rust. I split up schema because it made sense taxonomically in my mind 😊

I'm actually going to be adding another giant multi-armed function validate_value to schema.rs soon so a line limit doesn't make a whole lot of sense.

Also, I'm repurposing this PR to just the addition of validate_query + refactors, merge whenever you feel comfortable with the PR.

svmnotn commented 8 years ago

I think it would be better to split this PR into two, one that does the bulk of the changes that you have so far, and then one the impls the validate_query method.

Also just to note: Stop messing with the .gitignore, how many times do we have to have this discussion about you removing the windows' Desktop.ini file from it?

calebmer commented 8 years ago

I agree this probably should be two PRs, but I really don't want to do the git lifting work to split this PR into two 😊

calebmer commented 8 years ago

What's blocking getting this merged? I don't want to merge until you are totally on board.