-
Notifications
You must be signed in to change notification settings - Fork 34
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
Draft of the user admin #82
Conversation
I left it as a draft, since I didn't add any tests, and there is one comment in the code that I do not understand :( |
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.
Some minor details, although not all of them are necessary
.github/workflows/tests.yml
Outdated
@@ -36,41 +36,27 @@ jobs: | |||
python-version: [3.7, 3.8, 3.9] | |||
requirements-level: [pypi] | |||
cache-service: [redis] | |||
db-service: [postgresql10, postgresql13, mysql5, mysql8] | |||
db-service: [postgresql10, postgresql13] |
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.
@zzacharo, please check this change! The main issue is that mysql is not happy with the table of vocabularies
. If I get it right, the trigger for this is that now the module also depends in invenio-administration
, and that brings its dependencies (invenio-vocabularies
in particular). Note that neitherinvenio-administration
nor invenio-vocabularies
do not check the mysql pipelines. I have the feeling that they did, they would have the same issue.
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.
Following @jrcastro2 suggestion, another option would be to put something like inveniosoftware/invenio-vocabularies#261
❤️ Thank you for your contribution!
Description
Please describe briefly your pull request.
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Third-party code
If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: