-
Notifications
You must be signed in to change notification settings - Fork 0
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
Technical review #11
Comments
Sorry... clicked enter to fast... Working on more edits |
Let's have this as a first round of changes @mariostieriansys - please let me know if you need help addressing them |
@mariostieriansys @randallfrank - some of the points were not addressed. Please adapt or add an explanation on why they will not be addressed. Thanks! |
Hi @RobPasMue , sorry I was following your comments from the email, where I lost some of the points. I'll take care of them this morning |
Awesome, no problem at all! :) |
Hi @mariostieriansys - after performing the technical review of this package I have found a few elements that need to be addressed. Listing them below. Feel free to reach out if you need any clarification:
pyproject.toml
,setup.py
andsetup.cfg
? I believe that only the first one is needed because you are already providing your build information in it (as well as dev tools configuration). Can we get rid of thesetup.*
files?ansys-pre-commit-hook
for adding license headers. Since your repo is slightly different in terms of folders, you might have to adapt it according to https://pre-commit-hooks.docs.ansys.com/version/stable/#specify-directories-to-run-the-hook-onpre-commit autoupdate
and bump the hook versionsfiles: .
pyproject.toml
metadata - Python versions supported is not true** remove trailing colon
https://github.com/ansys-internal/ansys-tools-omniverse/blob/f4c05f857e34e5fae53c4c02aae6f6145b24fea0/exts/ansys.tools.omniverse.core/ansys/tools/omniverse/core/extension.py#L81
** missing types
https://github.com/ansys-internal/ansys-tools-omniverse/blob/f4c05f857e34e5fae53c4c02aae6f6145b24fea0/exts/ansys.tools.omniverse.core/ansys/tools/omniverse/core/extension.py#L243-L249
Other potential recommendations:
scripts
folder to avoid cluttering the root level of the repoThe text was updated successfully, but these errors were encountered: