-
Notifications
You must be signed in to change notification settings - Fork 34
feat: Add exercises tags #326
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
Conversation
@orronai
orronai
commented
Oct 5, 2021
- Added tags per course
- Added for the exercises.html template the tags
- Added the tables of the tags
- Added a test
- Added tags per course - Added for the exercises.html template the tags - Added the tables of the tags - Added a test
Codecov Report
@@ Coverage Diff @@ ## master #326 +/- ## ========================================== + Coverage 84.08% 84.36% +0.28% ========================================== Files 63 64 +1 Lines 2903 2955 +52 ========================================== + Hits 2441 2493 +52 Misses 462 462
Continue to review full report at Codecov.
|
- Added a bridge of tags in models folder - Added course and date_created column to the tags table - Added a constraint to the tags table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should find a better way. Creating these godlike functions with trillion parameters is a bad habit. Can you please find another way to do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really find a better way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same - we should try not to write such godlike functions
- Splitted a function into 3 functions - Changed some functions names
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.07%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe /exercises/tag/<tagname>? Or /tags/exercise/_ in case we'll want to tag other things in the future (users, maybe?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now think about changing this tablename to ExerciseTag, and the connection to ExerciseToTag, in order to allow easy future expansion of the tags to user. IDK what the right approach to a state where we have both Users and Exercises that use tags, but I have a strong hunch that they shouldn't share the same table.