-
Notifications
You must be signed in to change notification settings - Fork 11
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
[#315] Display UUID and version of any object type in the admin interface #332
Conversation
Make sure your PR description states: |
@ErhanCitil sync your PR-branches with master to trigger CI |
Codecov Report
@@ Coverage Diff @@
## master #332 +/- ##
==========================================
- Coverage 94.78% 94.67% -0.11%
==========================================
Files 131 132 +1
Lines 4543 4563 +20
==========================================
+ Hits 4306 4320 +14
- Misses 237 243 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@alextreme green |
Aanpak klopt volgens mij niet helemaal. Er dienen geen ObjectRecord relaties te worden toegevoegd. In plaats daarvan voeg een veld '_version' toe aan de (gecachede) ObjectType. In de clean methode van de ObjectType dient naast '_name' ook in '_version' de versie van het objecttype opgeslagen te worden. |
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.
Zie comment
@ErhanCitil yubikey lijkt me hier niet in te horen. Ook graag je objectrecord migraties weghalen. |
@ErhanCitil graag je objectrecord migraties weghalen. |
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.
zie migrations 0009 en 0010
@alextreme deze is ready to review |
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.
PR is messy due to the requirements upgrade and blacking of migrations, but the addition of _version seems correct.
@annashamray feel free to do a double-check, if this is not (yet) okay please discuss this with Erhan at the office
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.
3 meta puntjes:
- Kunnen we black pinnen in deze PR om hem zonder veel black wijzigingen akkoord te krijgen?
- Deze hele Pr graag in 2 commits: Black pinnen en 315 issue
- Nieuwe PR met Black update, pinnen op nieuwe versie en black uitgevoerd op alle bestanden.
Please rebase so only the relevant changes are shown. |
6101b37
to
e40065e
Compare
e40065e
to
1630bbc
Compare
@joeribekker @alextreme rebase gelukt |
implemented in #393 |
Fixes #315