Skip to content

Conversation

@goerz
Copy link
Collaborator

@goerz goerz commented May 31, 2025

Instructions for collaborators (including myself) on source code formatting, building the documentation, and running tests.

Added Makefile, since that's what I use for any kind of development workflow

@codecov
Copy link

codecov bot commented May 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.90%. Comparing base (a84a33d) to head (23a716b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   86.90%   86.90%           
=======================================
  Files           1        1           
  Lines          84       84           
=======================================
  Hits           73       73           
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CONTRIBUTING.md Outdated
Alternatively, if you are on Unix and have `make` installed, run `make codestyle`. See `make help` for details.


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so easy to just run JuliaFormatter so please, let's not add to many things that are really not superneeded.

Suggested change
Alternatively, if you are on Unix and have `make` installed, run `make codestyle`. See `make help` for details.

@mofeing
Copy link
Collaborator

mofeing commented Jun 1, 2025

I don't like having a Makefile when just doing ]test or julia --project=docs docs/make.jl it's so easy.

If they were more complicated settings or pipelines, but it's just one line in Julia.

@goerz
Copy link
Collaborator Author

goerz commented Jun 1, 2025

This is mostly for me. I use a Makefile for everything. That way, I don’t have to remember the particular invocation required to, e.g. run test with and without coverage. Does it hurt really hurt to have one?

@goerz
Copy link
Collaborator Author

goerz commented Jun 1, 2025

Also, though, I disagree pretty strongly, philosophically. I don't think contributors necessarily know about ]test, and julia --project=docs docs/make.jl isn't even universal (some of my own projects currently use test also as the environment to build the documentation, and have no docs/Project.toml). JuliaFormatter is even more obscure. Almost nobody knows how to apply project-specific formatting. A great majority of PRs I review have lines with trailing whitespace, and issues like that. And that's all before any kind of custom or unusual setup, which, admittedly, this project doesn't have. The first thing I look for when preparing a pull request for a project I'm not familiar with is for CONTRIBUTING.md for exactly this kind of information. When a project doesn't have any instructions for how to run tests, build documentation, or apply formatting, that's always a downer – and for something like JOSS (which I sometimes review for), these instructions are even a hard requirement for publication. Even as an "expert", I still struggle to figure out how to run tests on many more complicated projects. I don't think I ever fully figured out how to run specific tests in an accessible/debuggable way for Documenter, despite being an active collaborator, due to a lack of documentation for how the tests are organized and fit together on that project.

Plus, I can almost guarantee that if I ever took a break from working with Julia packages on a daily basis, after 6 months I wouldn't remember even things like ]test. I certainly had that experience with Python projects where I didn't write Python for a year or so and did not remember which invocation of tox would run the tests, or what exactly the build commands for Sphinx were. Everything like that, even if "trivial", should be noted somewhere in the project. Whether you need a Makefile, specifically, is more a matter of taste. I do everything with make, and if I'm working on any project at all (e.g., to prepare a PR), the first thing I when that project doesn't have a Makefile is to write myself one. If find make codestyle infinitely faster than opening up a REPL, loading JuliaFormatter, and then running format("."). Plus, a Makefile solves the "I don't remember how to invoke the specific tools in this language". Whether it's Julia, Python, C, Fortran or anything else, if there's a Makefile I know that make test is going to run the test suite, and in fact I can remind myself about what the equivalent manual invocation is by reading the Makefile.

I consider you the maintainer of this project, so I'm not going for force "tooling" on you, if you think a Makefile or any notes in CONTRIBUTING.md are a maintenance burden. No hard feeling if you decide not to merge this 😉. But I certainly need the Makefile for myself, if I'm doing anything on the project. If it's not committed to the repo, I'll still leave it around in my checkout. Admittedly, once we push out v1.0.0, I pretty much expect the project to be dormant. So maybe documenting any of the development workflows is a waste of time for a project that won't have much, if any, development going forward. I'd still consider it good style.

@goerz goerz force-pushed the mg/dev-instructions branch 2 times, most recently from dd340a6 to f8bd40c Compare June 13, 2025 15:01
@mofeing
Copy link
Collaborator

mofeing commented Jun 16, 2025

I see that we have different views on project management infrastructure, but this is my point: Makefiles are hard to maintain because nowadays very few people really understand their language. I'm thinking not only on us but on the people that can follow us. I can see the point of using them for multi-language repos, when you need to make them interact between several tools, but IMO, it doesn't fit well on modern repos where their functionality is really backed in on the standard tools.

My philosophy is that we shouldn't complexify a repo more than it really adds as a library.
This is a Julia package. Since it is a single-language project and a very simple one, we should go for trying to just use Julia tools and their standards or common practices. Using Makefile is not a common practice in Julia repos but using a docs/make.jl, while not standard, is kind of common practice.

I would expect that, since this is a Julia-only package, any contributor knows how to code in Julia and this most probably will include how to run ]test.

Finalizing, I'm not against adding this info to CONTRIBUTING.md. Reconsidering, I think it's a good idea 😊.

@goerz goerz force-pushed the mg/dev-instructions branch from f8bd40c to a652c05 Compare June 16, 2025 14:01
@goerz
Copy link
Collaborator Author

goerz commented Jun 16, 2025

That's fine. Maybe one day I can convince you of the virtues of a Makefile 😉

I still need it (basically, every folder on my hard drive contains a Makefile, whether it's a software project, paper, report, presentation; Typing make is deeply ingrained in my muscle memory and I never use any other method to run any kind of development task). But we don't have to commit it to master: I can keep just keep it in my local checkout, respectively on a personal branch.

I've updated this to only contain some basic instructions to CONTRIBUTING.md without mention of make now.

goerz added 2 commits June 16, 2025 10:18
Instructions for collaborators on source code formatting, building the
documentation, and running tests.

Added blue-style badge to README

Added `Makefile`
@goerz goerz force-pushed the mg/dev-instructions branch from a652c05 to 23a716b Compare June 16, 2025 14:18
@mofeing mofeing merged commit f621fd2 into master Jun 16, 2025
8 checks passed
@mofeing mofeing deleted the mg/dev-instructions branch June 16, 2025 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants