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

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

Open
RobotSail opened this issue Jun 10, 2024 · 11 comments
Labels

Comments

@RobotSail
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@bjhargrave
Copy link
Contributor

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
Copy link
Member Author

@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
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member Author

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

Why? They're simple, they work.

@russellb
Copy link
Member

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.

Copy link

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.

@github-actions github-actions bot added the stale label Aug 28, 2024
@nathan-weinberg
Copy link
Member

@RobotSail are you interested in doing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants