search

instructlab / dev-docs

Developer documents for the InstructLab organization
Apache License 2.0
2 stars 27 forks source link

Create targets in makefile to automatically install the pyspelling dependency if it doesn't exist #86

Open RobotSail opened 3 months ago

RobotSail commented 3 months ago

Currently in the Makefile, we have spellcheck as a target which calls the pyspelling tool.

When the tool isn't installed, running the target results in an error. It'd be nice if the makefile tried to automatically download it for you.

russellb commented 3 months ago

Currently in the Makefile, we have spellcheck as a target which calls the pyspelling tool.

When the tool isn't installed, running the target results in an error. It'd be nice if the makefile tried to automatically download it for you.

tox -e spellcheck would be a good way to make this easier

nathan-weinberg commented 3 months ago

Some good examples of this kind of implementation in https://github.com/instructlab/instructlab and https://github.com/instructlab/sdg

bjhargrave commented 3 months ago

I am not sure why we have Makefiles in any of the instructlab projects since they are primarily python projects. I know we need c/c++ tooling to build llama_cpp for the cli, but I suspect that is the exception and thus we shouldn't require users to have a make command present for the other projects.

RobotSail commented 3 months ago

@bjhargrave Dev-docs is primarily a doc repo with some tooling that happens to be implemented in Python. IMO I think it's fine to keep it as Makefile, especially because of how simple it is.

nathan-weinberg commented 3 months ago

@bjhargrave AFAIK none of the Makefile targets have anything of particular concern to most users. At most they allow for running CI checks that will happen on PRs anyway.

bjhargrave commented 3 months ago

I am questioning why they exist in the repos at all. At best they are duplicative of other processes which are used in the proper builds. And so they will get out of date as official processes change. They seem to be only a convenience for some maintainers (who are into Makefiles) rather then proper, maintained parts of the projects.

It seems like we need a policy on the use of Makefiles :-) Who are they for? What is their purpose if they are not part of the proper build processes? Or should they be part of the proper build processes so that we can know they are always valid and up-to-date. Should there be consistency across repos in the provided targets?

nathan-weinberg commented 3 months ago

To be honest, I feel like it's a non-issue - but if you want to open up an issue in this repo for that to continue the discussion, feel free. I don't want to derail @RobotSail's original issue here.

RobotSail commented 3 months ago

It seems like we need a policy on the use of Makefiles

Why? They're simple, they work.

russellb commented 3 months ago

It seems like we need a policy on the use of Makefiles

Why? They're simple, they work.

This is pretty much it. Shortcuts for dev workflow things, mostly.

Alternative proposals are fine, though this approach is in almost every repo already.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had activity within 90 days. It will be automatically closed if no further activity occurs within 30 days.