Feature/leden import - Githubissues

WISVCH / dienst2

CH Dienstensysteem: administratie van leden en post

https://dienst2.ch.tudelft.nl

Other

4 stars 2 forks source link

Feature/leden import #240

Closed Fastjur closed 5 years ago

Fastjur commented 5 years ago

I think this is ready to be reviewed.

[ ] I have an issue where locally the search function does not work. Could this be related to my changes? Someone should test this locally

Fastjur commented 5 years ago

@praseodym @jgadelange So. What do we want to do with the fields. Make them nullable or not? It would make more sense in my eyes, unless there is a good reason that we shouldn't do this.

praseodym commented 5 years ago

@praseodym @jgadelange So. What do we want to do with the fields. Make them nullable or not? It would make more sense in my eyes, unless there is a good reason that we shouldn't do this.

I oppose converting any fields as nullable unless required by a unique constraint. There should be no semantic difference between blank and null, so I do not see a reason to have both options in a single field.

Fastjur commented 5 years ago

Woop tie doo. Maybe we should squash merge this. Many very small commits :D

praseodym commented 5 years ago

Maybe we should squash merge this. Many very small commits :D

I'd prefer a squash of some of the commits in this branch, so that we that we can do a regular no-ff merge instead. I already mentioned this in a previous comment: https://github.com/WISVCH/dienst2/pull/240#discussion_r299860168