rapidsai / dependency-file-generator

https://pypi.org/project/rapids-dependency-file-generator/
Apache License 2.0
15 stars 13 forks source link

Add more tests #10

Closed ajschmidt8 closed 2 years ago

ajschmidt8 commented 2 years ago

This PR includes the following changes:

vyasr commented 2 years ago

@ajschmidt8 I just got around to reviewing and realized you had merged! In any case everything looks good and I didn't have much to comment on here. The only recommendations I would make (not necessarily in scope for this PR) are:

ajschmidt8 commented 2 years ago

@ajschmidt8 I just got around to reviewing and realized you had merged! In any case everything looks good and I didn't have much to comment on here. The only recommendations I would make (not necessarily in scope for this PR) are:

  • We should rename the main function in rapids_dependency_file_generator.py and reserve that for the CLI entrypoint in cli.py.
  • We should start adding some documentation soon. Doesn't have to be fancy linted docstrings etc, just some basics for devs to get oriented when they first start looking through.

Sorry! Just trying to keep things moving since the GH Actions migrations depend on this work. I agree with both of these items.

For the second item, I would argue that we should have these docstrings linted. If RAPIDS is going to depend on this tool, we need to legitimize it as much as possible. I'm not super familiar with Python coding conventions, so if we can programmatically enforce best practices, I think that's a good idea.