Conversation
There was a problem hiding this comment.
Hi Nico,
thanks for the PR, I had a first glance. Are you on our slack too?
Couple of comments:
- Why did you delete docker-compose.dev?
- Please revert changes to the backup, deploy.sh and the yml configs.
- I will add more comments to the source
- Please consider adding a logger (import logging, logger = logging.getLogger(name)
|
tonight :) |
|
This concerns #226 |
| migrations.CreateModel( | ||
| name='StudentEvaluation', | ||
| fields=[ | ||
| ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), |
There was a problem hiding this comment.
I think in this project we have never used verbose_name before. This usage here is straight-forward, but it could create confusion with the additional translation layer later on and the separate setting of labels in the form class. I'm thinking about whether this is helping or complicating the understanding in other code files.
backend/apps/evaluation/templates/evaluation/studentevaluation_form.html
Outdated
Show resolved
Hide resolved
@maltezacharias Does one logger for each file make sense? I would add one globally like |
|
Am I the only one getting a |
Made base Model abstract; registered Model
|
|
||
| operations = [ | ||
| migrations.AlterField( | ||
| model_name='newsletter', |
There was a problem hiding this comment.
Why do you add a migration for newsletter?
There was a problem hiding this comment.
That was unintentional, Ill fix it in the next commits.
backend/apps/evaluation/forms.py
Outdated
| 'wirst?'), | ||
| 'contact_mail': _('Hier kannst du deine Mail da lassen, falls du uns für Rückfragen zur Verfügung stehen ' | ||
| 'möchtest'), | ||
| 'communication_with_institutions': _('Wie lief für dich die Kommunikation mit den Institutionen?'), |
There was a problem hiding this comment.
Aren't these questions a bit too open to get helpful answers?
There was a problem hiding this comment.
Those are the questions initially suggested in the typeform (see slack). I too think that it might be helpful to be more specific, but I havent really spent a lot of time to think about possible alternatives – so I'm open to any suggestions.
There was a problem hiding this comment.
When can query statistics like how many hospitals contacted a student on average etc. but we don't know anything about the process after they were contacted. Maybe something like "How clear were the requirements of the hospitals?", "Did you receive offers which were interesting for you?"...
|
Please run Please also address @bjrne's and @maltezacharias's comments and/or resolve them. |
Ja, einer pro File macht absolut Sinn, EDIT: |
|
The missing translations block this PR now |
|
Are you still at this @n-hackert ? :) |
5 participants
Moin ihr Lieben,
ich hatte über Ostern ein bisschen Zeit, um mal meine Python-"Skills" wieder aufzufrischen und habe mich in Django eingelesen. Da ich mit Fabi ja mal an den Feedback-Typeforms gearbeitet habe, hab ich einfach mal versucht, damit anzufangen, die in die Codebase zu integrieren. Ich würde mich freuen, wenn sich das jemand von euch kurz anschauen und n kurzes Statement dazu abgeben könnte, ob ihr den Ansatz sinnvoll findet. (Is auch nicht viel Code :D)
Liebe Grüße und noch einen schönen Ostermontag!
Nico