Skip to content
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

Review of initial commit #2

Open
PeterC-DLS opened this issue Jun 28, 2021 · 0 comments
Open

Review of initial commit #2

PeterC-DLS opened this issue Jun 28, 2021 · 0 comments

Comments

@PeterC-DLS
Copy link

A quick review shows some issues to be addressed:

  • NeXusOntology_V1.1.py should not have the version in its name. Derive version from branch or constant, instead.
  • Remove the notebook as it duplicates the script.
  • FedIDs should be removed L23-4
  • Reading non-existent files will fail L113-6
  • Use endswith L166 and later lines
  • Append to list instead of combining new lists L167 and later lines
  • Use a string's join method L180
  • Use temporary variable instead of repeatedly dereferencing multiple dictionaries L188-207
  • Allow GH token to be passed as argument or obtained from the shell environment
  • Split script to functions
  • Avoid use of wildcard import on L342
  • Reformat string in L419
  • Do not run test code L534
  • Remove extraneous comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant